[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-17 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

Hi @probinson @dblaikie  @aprantl , I've was investigating and working on your 
inputs regarding the problem with DW_at_defaulted once. I think clang has also 
some issues. Though I'm not able to precisely point out. I ranned into some 
problems in CFE while marking out_of_class functions. i.e consider this for 
instance "debug-info-defaulted-out_of_class.cpp" FIXME:.  Causing too much 
trouble and possibly can introduce some bugs in clang/llvm. 
May be we'll reconsider this in future. Thanks for putting time in reviewing 
and discussing this.

Anyway here's the next course of action I'm considering. I'll be marking this 
review as abandoned. I'll raise a fresh new review for DW_AT_deleted feature, 
that;s pretty straight forward, considering C++ spec. If it's deleted, it has 
to be declared/defined{whatever} deleted in it's first declaration. This 
feature also augments the consumer information regarding which spl member 
functions are deleted, hence restrict their calling for user or do something 
useful with this at it's disposal.

@probinson @dblaikie @aprantl Please share your thoughts/comments inclination 
regarding adding this DW_AT_deleted to clang/llvm.


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

https://reviews.llvm.org/D68117



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


[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added inline comments.



Comment at: lld/ELF/Options.td:361
 
+def time_trace: F<"time-trace">, HelpText<"Record time trace">;
+

aganea wrote:
> Given this option is a candidate for the other LLD drivers, I am wondering if 
> we couldn't have a shared `lld/Common/CommonOpt.td`. @ruiu WDYT?
In many cases command line options are slightly different among drivers. so 
currently we have a completely separated option file for each driver. It looks 
like maintaining each file separately makes things much easier, so I wouldn't 
create a new common file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69043



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


[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

2019-10-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision.
Charusso added a comment.
This revision is now accepted and ready to land.

When you see

  static bool traverseHiddenNodes(
  const ExplodedNode *N,
  llvm::function_ref PreCallback,
  llvm::function_ref PostCallback,
  llvm::function_ref Stop) {

that is 100% will produce errors, as it wants to be so smart. I think it is a 
very bad design from the beginning, so it will be still bugprone.

Other than that now every node makes sense, thanks!




Comment at: clang/test/Analysis/dump_egraph.c:46
 
 // CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, 
\"file\": \"{{(.+)}}dump_egraph.c\" \}
 

`\"file\": \"{{(.+)}}dump_egraph.c\" \}` is a cool workaround, but otherwise if 
you do not want to end line 32, could you remove this line's ending please?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69150



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


[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

2019-10-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:325
 if (getKind() != CE_CXXAllocator)
   if (isArgumentConstructedDirectly(Idx))
 if (auto AdjIdx = getAdjustedParameterIndex(Idx))

What about this one? It smells the same issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69155



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


[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment.

I applied this patch and built clang with ThinLTO. Here is the generated file:

$ less bin/clang-10.json 
{"traceEvents":[{"pid":1,"tid":185412,"ph":"X","ts":181,"dur":1441864,"name":"Parse
 input 
files","args":{"detail":""}},{"pid":1,"tid":185412,"ph":"X","ts":12564626,"dur":541606,"name":"OptModule","args":{"detail":"ld-temp.o"}},{"pid":1,"tid":185412,"ph":"X","ts":1454232,"dur":17946238,"name":"LTO","args":{"detail":""}},{"pid":1,"tid":185412,"ph":"X","ts":19461534,"dur":15275,"name":"GC","args":{"detail":""}},{"pid":1,"tid":185412,"ph":"X","ts":19721702,"dur":1241569,"name":"Write
 output 
file","args":{"detail":""}},{"pid":1,"tid":185412,"ph":"X","ts":2,"dur":20963276,"name":"ExecuteLinker","args":{"detail":""}},{"pid":1,"tid":185413,"ph":"X","ts":0,"dur":20963276,"name":"Total
 ExecuteLinker","args":{"count":1,"avg 
ms":20963}},{"pid":1,"tid":185414,"ph":"X","ts":0,"dur":17946237,"name":"Total 
LTO","args":{"count":1,"avg 
ms":17946}},{"pid":1,"tid":185415,"ph":"X","ts":0,"dur":1441864,"name":"Total 
Parse input files","args":{"count":1,"avg 
ms":1441}},{"pid":1,"tid":185416,"ph":"X","ts":0,"dur":1241569,"name":"Total 
Write output file","args":{"count":1,"avg 
ms":1241}},{"pid":1,"tid":185417,"ph":"X","ts":0,"dur":541704,"name":"Total 
OptModule","args":{"count":2,"avg 
ms":270}},{"pid":1,"tid":185418,"ph":"X","ts":0,"dur":15274,"name":"Total 
GC","args":{"count":1,"avg 
ms":15}},{"cat":"","pid":1,"tid":0,"ts":0,"ph":"M","name":"process_name","args":{"name":"ld.lld"}}]}

This file seems much smaller than yours. What am I missing? (I couldn't 
download your file because the server times out perhaps due to the file size.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69043



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


[PATCH] D69075: [WebAssembly] -pthread implies -target-feature +sign-ext

2019-10-17 Thread Thomas Lively via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
tlively marked an inline comment as done.
Closed by commit rG807cecad5d98: [WebAssembly] -pthread implies -target-feature 
+sign-ext (authored by tlively).

Changed prior to commit:
  https://reviews.llvm.org/D69075?vs=225331=225569#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69075

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


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -48,11 +48,11 @@
 
 // Thread-related command line tests.
 
-// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, and 
--shared-memory
+// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, +sign-ext, and 
--shared-memory
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fuse-ld=wasm-ld -pthread 2>&1 \
 // RUN:  | FileCheck -check-prefix=PTHREAD %s
-// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" 
"-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals"
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" 
"-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals" 
"-target-feature" "+sign-ext"
 // PTHREAD: wasm-ld{{.*}}" "-lpthread" "--shared-memory"
 
 // '-pthread' not allowed with '-mno-atomics'
@@ -73,6 +73,12 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_MUT_GLOBALS %s
 // PTHREAD_NO_MUT_GLOBALS: invalid argument '-pthread' not allowed with 
'-mno-mutable-globals'
 
+// '-pthread' not allowed with '-mno-sign-ext'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -pthread -mno-sign-ext 2>&1 \
+// RUN:   | FileCheck -check-prefix=PTHREAD_NO_SIGN_EXT %s
+// PTHREAD_NO_SIGN_EXT: invalid argument '-pthread' not allowed with 
'-mno-sign-ext'
+
 // '-fwasm-exceptions' sets +exception-handling
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fwasm-exceptions 2>&1 \
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -141,7 +141,7 @@
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
 
-  // '-pthread' implies atomics, bulk-memory, and mutable-globals
+  // '-pthread' implies atomics, bulk-memory, mutable-globals, and sign-ext
   if (DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread,
  false)) {
 if (DriverArgs.hasFlag(options::OPT_mno_atomics, options::OPT_matomics,
@@ -159,12 +159,19 @@
   getDriver().Diag(diag::err_drv_argument_not_allowed_with)
   << "-pthread"
   << "-mno-mutable-globals";
+if (DriverArgs.hasFlag(options::OPT_mno_sign_ext, options::OPT_msign_ext,
+   false))
+  getDriver().Diag(diag::err_drv_argument_not_allowed_with)
+  << "-pthread"
+  << "-mno-sign-ext";
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+atomics");
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+bulk-memory");
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+mutable-globals");
+CC1Args.push_back("-target-feature");
+CC1Args.push_back("+sign-ext");
   }
 
   if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -48,11 +48,11 @@
 
 // Thread-related command line tests.
 
-// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, and --shared-memory
+// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, +sign-ext, and --shared-memory
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fuse-ld=wasm-ld -pthread 2>&1 \
 // RUN:  | FileCheck -check-prefix=PTHREAD %s
-// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" "-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals"
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" "-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals" "-target-feature" "+sign-ext"
 // PTHREAD: wasm-ld{{.*}}" "-lpthread" "--shared-memory"
 
 // '-pthread' not allowed with '-mno-atomics'
@@ -73,6 +73,12 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_MUT_GLOBALS %s
 // PTHREAD_NO_MUT_GLOBALS: invalid argument '-pthread' not allowed with '-mno-mutable-globals'
 
+// '-pthread' not allowed with '-mno-sign-ext'
+// 

r375199 - [WebAssembly] -pthread implies -target-feature +sign-ext

2019-10-17 Thread Thomas Lively via cfe-commits
Author: tlively
Date: Thu Oct 17 21:34:26 2019
New Revision: 375199

URL: http://llvm.org/viewvc/llvm-project?rev=375199=rev
Log:
[WebAssembly] -pthread implies -target-feature +sign-ext

Summary:
The sign extension proposal was motivated by a desire to not have
separate sign-extending atomic operations, so it is meant to be
enabled when threads are used.

Reviewers: aheejin, dschuff

Subscribers: sbc100, jgravelle-google, sunfish, jfb, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/test/Driver/wasm-toolchain.c

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=375199=375198=375199=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Thu Oct 17 21:34:26 2019
@@ -141,7 +141,7 @@ void WebAssembly::addClangTargetOptions(
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
 
-  // '-pthread' implies atomics, bulk-memory, and mutable-globals
+  // '-pthread' implies atomics, bulk-memory, mutable-globals, and sign-ext
   if (DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread,
  false)) {
 if (DriverArgs.hasFlag(options::OPT_mno_atomics, options::OPT_matomics,
@@ -159,12 +159,19 @@ void WebAssembly::addClangTargetOptions(
   getDriver().Diag(diag::err_drv_argument_not_allowed_with)
   << "-pthread"
   << "-mno-mutable-globals";
+if (DriverArgs.hasFlag(options::OPT_mno_sign_ext, options::OPT_msign_ext,
+   false))
+  getDriver().Diag(diag::err_drv_argument_not_allowed_with)
+  << "-pthread"
+  << "-mno-sign-ext";
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+atomics");
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+bulk-memory");
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+mutable-globals");
+CC1Args.push_back("-target-feature");
+CC1Args.push_back("+sign-ext");
   }
 
   if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=375199=375198=375199=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Thu Oct 17 21:34:26 2019
@@ -48,11 +48,11 @@
 
 // Thread-related command line tests.
 
-// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, and 
--shared-memory
+// '-pthread' sets +atomics, +bulk-memory, +mutable-globals, +sign-ext, and 
--shared-memory
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fuse-ld=wasm-ld -pthread 2>&1 \
 // RUN:  | FileCheck -check-prefix=PTHREAD %s
-// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" 
"-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals"
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics" 
"-target-feature" "+bulk-memory" "-target-feature" "+mutable-globals" 
"-target-feature" "+sign-ext"
 // PTHREAD: wasm-ld{{.*}}" "-lpthread" "--shared-memory"
 
 // '-pthread' not allowed with '-mno-atomics'
@@ -73,6 +73,12 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_MUT_GLOBALS %s
 // PTHREAD_NO_MUT_GLOBALS: invalid argument '-pthread' not allowed with 
'-mno-mutable-globals'
 
+// '-pthread' not allowed with '-mno-sign-ext'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -pthread -mno-sign-ext 2>&1 \
+// RUN:   | FileCheck -check-prefix=PTHREAD_NO_SIGN_EXT %s
+// PTHREAD_NO_SIGN_EXT: invalid argument '-pthread' not allowed with 
'-mno-sign-ext'
+
 // '-fwasm-exceptions' sets +exception-handling
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fwasm-exceptions 2>&1 \


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


[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: clang/test/Format/dry-run-alias.cpp:1-2
+// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr
+// RUN: grep -E "*code should be clang-formatted*" %t.stderr
+

hliao wrote:
> Why not check with `FileCheck`?
Absolutely no real reason other than my lack of ability to get it working 
correctly. I was trying to follow the other examples of how it could be written 
with FIleCheck but failed.  In the end, I copied how it was done in some of the 
other tests for now but If someone could help me I would love to change them to 
use FileCheck properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Hideki Saito via Phabricator via cfe-commits
hsaito added a comment.

In D69088#1714019 , @Meinersbur wrote:

> In D69088#1713933 , @hsaito wrote:
>
> > Have we established general consensus for the desire to have the flexible 
> > enough loop optimization pass ordering to accomplish the outcome of the new 
> > directive, and shared vision for the path to get there? If we are making 
> > this a general clang directive, I'd like to see the vision to get there w/o 
> > depending on polly. If this is already discussed and settled, pointer to 
> > that is appreciated so that I can learn.
>
>
> Response to the RFCs was meager. However, I got positive feedback at various 
> conferences, including last year's DevMtg where my version for loop 
> transformations was a technical talk .


Personally, I like the intent. I don't foresee a clear (enough) path to get 
there. This leads to hesitation of adding a new non-experimental pragma and 
present it to programmers. If you call it experimental, it's easier for me to 
swallow.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713933 , @hsaito wrote:

> Have we established general consensus for the desire to have the flexible 
> enough loop optimization pass ordering to accomplish the outcome of the new 
> directive, and shared vision for the path to get there? If we are making this 
> a general clang directive, I'd like to see the vision to get there w/o 
> depending on polly. If this is already discussed and settled, pointer to that 
> is appreciated so that I can learn.


Response to the RFCs was meager. However, I got positive feedback at various 
conferences, including last year's DevMtg where my version for loop 
transformations was a technical talk .


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713915 , @ABataev wrote:

> Just do not allow this form with respect_order clause.


What exactly would be the rules what is allowed and what isn't?

We can just not allow not mixing `#pragma clang loop` and `#pragma clang 
transform`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.

This is the bug that I'll be using in the LLVM Developer's Meeting presentation 
as an example of how to debug things, so //spoiler alert!//

And let me explain it a bit slower than usual.

Here's the original false positive that we're fixing:

F10304929: bad.html 

It has appeared by applying the Static Analyzer to LLVM itself. Namely, you 
should be able to reproduce it locally by applying the static analyzer to LLVM 
rL373385 ; the static analyzer itself should 
be one commit behind //this// commit.

The warning tells us that variable `Reg` on line 824 is uninitialized:

F10304965: Screen Shot 2019-10-17 at 6.18.06 PM.png 


This, however, is clearly a false positive, because the lambda invocation 
`isKilledReg(MO, Reg)` receives a non-const reference to `Reg` and initializes 
it on the current path:

F10304987: Screen Shot 2019-10-17 at 6.19.32 PM.png 


The static analyzer somehow fails to realize that `Reg` is initialized, and 
even displays a clearly incorrect note (41) "Returning without writing to 
'Reg'".

The variable `Reg` itself is declared in the caller function, 
`transferSpillOrRestoreInst`, and passed into the current function by 
reference, which is also named `Reg`:

F10305034: Screen Shot 2019-10-17 at 6.22.45 PM.png 


F10305046: Screen Shot 2019-10-17 at 6.24.04 PM.png 


In order to debug the issue, I dumped the trimmed exploded graph for the 
current analysis and used exploded-graph-rewriter to remove the unnecessary 
details:

  $ clang ... -analyze-function="(anonymous 
namespace)::LiveDebugValue::runOnMachineFunction(class llvm::MachineFunction 
&)" -analyzer-dump-egraph=graph.dot -trim-egraph
  
  $ exploded-graph-rewriter.py --topology graph.dot
  
  # I searched the topology dump and found out that our bug node is 52419.
  $ exploded-graph-rewriter.py --to 52419 --single-path --diff graph.dot

Here's the final graph dump:

F10305539: tmpruvE5U.html 

I tried to find out which transition in the graph was incorrect.

First of all I confirmed that there is no binding in the Store for the `Reg` in 
the bug state. The //referece// `Reg` was there and was known to refer to the 
//variable// `Reg`, but the variable itself is indeed nowhere to be found, as 
if it was never written to:

F10305142: Screen Shot 2019-10-17 at 6.35.33 PM.png 


This gave me confidence that the checker is not to be blamed: the variable was 
"known" to be uninitialized, therefore the checker was right to report it, and 
we need to find out why was the variable incorrectly known to be uninitialized.

Therefore, logically, the next question was why did the assignment on line 812, 
before step 41, was not recorded in the Store. Here's the ExplodedNode in which 
the assignment operator was evaluated:

F10305210: Screen Shot 2019-10-17 at 6.42.28 PM.png 


Here we can see that the assignment was in fact recorded in the Store, but in a 
wrong memory region. In our notation, `SymRegion{reg_$3519}` is an //unknown// region of memory that is the pointee of the reference 
`Reg` points to.

However, given that `Reg` is a parameter in a function call into which we dived 
during analysis, this symbolic value should not have existed to begin with, 
because we have concrete knowledge about what `Reg` points to. Instead, we 
should have had a binding in the Store from `Reg` (the memory region of the 
reference parameter) to `` (the pointer to the variable `Reg`), and loading 
from `Reg` (as part of evaluating which location should be written into) should 
have yielded `` rather than `{reg_$3519}`.

In order to confirm this educated guess, I compared the evaluation that the 
Static Analyzer did for call sites of `isLocationSpill()`:

F10305268: Screen Shot 2019-10-17 at 6.56.29 PM.png 


and `isKilledReg()`:

F10305274: Screen Shot 2019-10-17 at 6.57.33 PM.png 


And, indeed, the binding `Reg : ` is supposed to be there in both cases, 
but it is missing in case of `isKilledReg()`. Interestingly, the binding for 
the other parameter, `MO`, is also missing in case of `isKilledReg`.

By scrolling up a few nodes we can see that the bindings for //expressions// 
`Reg` and `MO` are present in the Environment when the call is being entered:

F10305300: Screen Shot 2019-10-17 at 7.01.05 PM.png 


In other words, the translation of parameter values from 

[PATCH] D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output

2019-10-17 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk added a comment.

Thanks for the comment @jkorous.

> I think you could've just used CHECK-DAG to fix the tests. It *might* be a 
> bit more robust. Although just reordering checks seems perfectly fine too. 
> https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive. 
> Using std::set here sounds reasonable to me but I'd like @arphaman to be 
> aware of this.

Thanks for pointing me to it - I have a minor preference towards a `sorted` 
order of outputs vs `order in which we visit the files`. I think a sorted order 
is much more easy to reason about for a client. Having said that, @arphaman 
what do you think? If you and Jan both feel that we should maintain the current 
order, I'll stick to that and not change `Dependencies` to `set`.

> BTW should we change std::vector Dependencies; in 
> DependencyCollector to set too?

I think this would change the output of `*.d` file? If so I'm a little hesitant 
to make that change because of the impact it would have, but once again I'll 
defer to your judgement on this (I'm very new to LLVM codebase).

> Also, I'm wondering why is the behavior different on Windows.

I confirmed that there are duplicate blacklist.txt files without changing 
`vector` to `set`. I'm setting up a Windows machine to try to figure out why 
that is the case. Will share what I find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69090



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


[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

2019-10-17 Thread Nicholas Allegra via Phabricator via cfe-commits
comex updated this revision to Diff 225557.
comex marked 2 inline comments as done.
comex added a comment.

So, I landed this patch but had to revert it as it broke the build on MSVC (and 
only MSVC).  That was almost a month ago, but I haven't gotten back around to 
this until now – sorry :|

In this updated patch, I switched is_random_iterator from a constexpr function:

  template 
  constexpr bool is_random_iterator() {
return std::is_same<
  typename std::iterator_traits::iterator_category,
  std::random_access_iterator_tag>::value;
  }

to a type alias:

  template 
  using is_random_iterator =
std::is_same::iterator_category,
 std::random_access_iterator_tag>;

and changed the uses accordingly.  For some reason, MSVC thought the two 
overloads of `drop_begin`, one with `enable_if()>` and 
one with `enable_if()>`, were duplicate definitions.  But 
with `is_random_iterator` changed to a type alias, MSVC is fine with them.  GCC 
and Clang think both versions are valid, so I think it's just an MSVC bug.

Simplified example for reference: https://gcc.godbolt.org/z/niXCy4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67647

Files:
  clang/include/clang/AST/Stmt.h
  clang/lib/Analysis/Consumed.cpp
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/iterator_range.h

Index: llvm/include/llvm/ADT/iterator_range.h
===
--- llvm/include/llvm/ADT/iterator_range.h
+++ llvm/include/llvm/ADT/iterator_range.h
@@ -20,9 +20,15 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 
+template 
+using is_random_iterator =
+  std::is_same::iterator_category,
+   std::random_access_iterator_tag>;
+
 /// A range adaptor for a pair of iterators.
 ///
 /// This just wraps two iterators into a range-compatible interface. Nothing
@@ -59,11 +65,31 @@
   return iterator_range(std::move(p.first), std::move(p.second));
 }
 
+/// Non-random-iterator version
+template 
+auto drop_begin(T &, int n) ->
+  typename std::enable_if::value,
+  iterator_range>::type {
+  auto begin = adl_begin(t);
+  auto end = adl_end(t);
+  for (int i = 0; i < n; i++) {
+assert(begin != end);
+++begin;
+  }
+  return make_range(begin, end);
+}
+
+/// Optimized version for random iterators
 template 
-iterator_range()))> drop_begin(T &,
-  int n) {
-  return make_range(std::next(adl_begin(t), n), adl_end(t));
+auto drop_begin(T &, int n) ->
+  typename std::enable_if::value,
+  iterator_range>::type {
+  auto begin = adl_begin(t);
+  auto end = adl_end(t);
+  assert(end - begin >= n && "Dropping more elements than exist!");
+  return make_range(std::next(begin, n), end);
 }
+
 }
 
 #endif
Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1573,6 +1573,14 @@
 }
 template  constexpr T *to_address(T *P) { return P; }
 
+template 
+auto index(R &,
+  typename std::iterator_traits>::difference_type N)
+  -> decltype(TheRange.begin()[N]) {
+  assert(N < TheRange.end() - TheRange.begin() && "Index out of range!");
+  return TheRange.begin()[N];
+}
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_STLEXTRAS_H
Index: clang/lib/Analysis/Consumed.cpp
===
--- clang/lib/Analysis/Consumed.cpp
+++ clang/lib/Analysis/Consumed.cpp
@@ -494,8 +494,10 @@
   void checkCallability(const PropagationInfo ,
 const FunctionDecl *FunDecl,
 SourceLocation BlameLoc);
-  bool handleCall(const CallExpr *Call, const Expr *ObjArg,
-  const FunctionDecl *FunD);
+
+  using ArgRange = llvm::iterator_range;
+  bool handleCall(const Expr *Call, const Expr *ObjArg,
+  ArgRange args, const FunctionDecl *FunD);
 
   void VisitBinaryOperator(const BinaryOperator *BinOp);
   void VisitCallExpr(const CallExpr *Call);
@@ -608,22 +610,21 @@
 // Factors out common behavior for function, method, and operator calls.
 // Check parameters and set parameter state if necessary.
 // Returns true if the state of ObjArg is set, or false otherwise.
-bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg,
+bool ConsumedStmtVisitor::handleCall(const Expr *Call,
+ const Expr *ObjArg,
+ ArgRange Args,
  const FunctionDecl *FunD) {
-  unsigned Offset = 0;
-  if (isa(Call) && isa(FunD))
-Offset = 1;  // first argument is 'this'
-
   // check explicit parameters
-  for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
+  unsigned Index = 0;
+  for (const Expr *Arg : Args) {
 // Skip variable argument lists.
-if 

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

2019-10-17 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk added a comment.

> I haven't looked into this in detail but it feels kind of wasteful to start a 
> process just to get this value.

Right, that's why I cache this information. So multiple compile commands 
sharing the same compiler process will trigger at 1 subprocess and then 
subsequently use the cached information.

>   Can't we just expose it in some of the clang libs we already link against?

The problem is, the value of `resource-dir` depends on `CLANG_VERSION` and 
`ARCH` #define's, that's set when the clang binary is built. So lets say you 
use `/home/kousikk/clang/7.0/bin/clang`, then if your compilation database is:

  [
{ "command": "/home/kousikk/clang/7.0/bin/clang -c /home/kousikk/test.cpp",
  "file": "/home/kousikk/test.cpp",
  "directory": "/home/kousikk/"
}
  ]

then the value of `-resource-dir` for compilation of `test.cpp` must be 
something like `/home/kousikk/clang/7.0/lib64/clang/7.0.0/`. Any libs used as 
part of building `clang-scan-deps` itself is going to point to its own 
resource-directory which is NOT the behaviour we want.

---

> could we please add support for generic -extra-arg, which can then add 
> -resource-dir like the other clang tools?

@arphaman would this be an option to `clang-scan-deps`? If so it might not work 
since commands in the compilation-database can be using more than 1 version of 
clang?

In addition, every project on which we use scan-deps will inturn have to 
specify this extra argument - wouldn't it be better if they didn't have to do 
that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69122



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Hideki Saito via Phabricator via cfe-commits
hsaito added a comment.

Have we established general consensus for the desire to have the flexible 
enough loop optimization pass ordering to accomplish the outcome of the new 
directive, and shared vision for the path to get there? If we are making this a 
general clang directive, I'd like to see the vision to get there w/o depending 
on polly. If this is already discussed and settled, pointer to that is 
appreciated so that I can learn.

Thanks,
Hideki


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


Re: Zorg migration to GitHub/monorepo

2019-10-17 Thread Galina Kistanova via cfe-commits
Hello everyone,

The build bot is almost ready to move to github.

As the next step we would migrate the staging master to listen for the both
changes, from SVN as it was before, and from github.

Tonight I am going to update the staging (aka silent) master and it will
start working with github.

I will send an email when it is done.

If you bots use one of the ported build factories from the list in my
previous e-mail, please feel free to stage them for working with
monorepo/github and once you are happy, let me know, and I'll configure
them accordingly on the production master. Once that's done, you could move
the bots back to the production master and you are done.

When you stage your bot, please follow these steps:
1. Stop your bot between builds, if possible,
2. Remove the bot working directory (usually this directory has a builder
name and is under directory where your bot is installed; if you are not
sure, check the 'builddir' param of your bot in
zorg/buildbot/osuosl/master/config/builders.py),
3. Edit buildbot.tac in the directory where your bot is installed. Change
"port = 9990" line to "port = 9994". Save the change.
4. Start your bot, make sure it connects to the staging master.
5. Send me a mail with the staged bot names.

Once you are happy with your bot building monorepo changes from github,
please send me an e-mail and I'll respond with the instructions of how to
get your bot back to production.

Please be aware that staging master could restart often. Please let me know
if you are having long running builds.

Feel free to ask if you have questions.
Please me know if you will see issues with the staging master.

Thanks

Galina

On Mon, Oct 14, 2019 at 6:16 PM Galina Kistanova 
wrote:

> Hello everyone,
>
>
>
> We are in the middle of porting the majority of zorg to
> GitHub/monorepo. The following build factories will be ported and if you
> use one of those for your bots, you are all covered:
>
>
>
> * ClangBuilder.getClangCMakeBuildFactory (31 bots)
>
> * ClangBuilder.getClangCMakeGCSBuildFactory (2 bots)
>
> * LibcxxAndAbiBuilder (23 bots)
>
> * SphinxDocsBuilder (7 bots)
>
> * UnifiedTreeBuilder (11 bots)
>
> * ABITestsuitBuilder (1 bot) - based on UnifiedTreeBuilder
>
> * ClangLTOBuilder (2 bots) - based on UnifiedTreeBuilder
>
> * LLDPerformanceTesuiteBuilder (1 bot) - based on UnifiedTreeBuilder
>
>
>
> Some build factories will be deprecated. If you use one of these, please
> change your bot to use something else instead. Here is the list of
> deprecated build factories:
>
>
>
> * ClangBuilder.getClangBuildFactory (0 bots)
>
> * LLDBuilder (0 bots)
>
> * ClangAndLLDBuilder (0 bots)
>
>
>
> However, some special build factories and build factories with a few bots
> would need your attention.
>
> Here is the list of build factories in need of porting. Patches are
> welcome.
>
>
>
> * LLVMBuilder (3 bots)
>
> * PollyBuilder (3 bots)
>
> * LLDBBuilder (6 bots)
>
> * SanitizerBuilder (10 bots)
>
> * CUDATestsuiteBuilder (1 bot) - depends on
> ClangBuilder.getClangBuildFactory
>
> * AOSPBuilder (1 bot) - depends on PollyBuilder
>
> * AnnotatedBuilder (2 bots)
>
> * OpenMPBuilder (2 bots)
>
> * FuchsiaBuilder (1 bot)
>
>
>
> Please feel free to ask if you have questions.
>
>
>
> Thanks
>
>
>
> Galina
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao added inline comments.



Comment at: clang/test/Format/dry-run-alias.cpp:1-2
+// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr
+// RUN: grep -E "*code should be clang-formatted*" %t.stderr
+

Why not check with `FileCheck`?



Comment at: clang/test/Format/dry-run.cpp:1-2
+// RUN: clang-format -style=LLVM -i --dry-run %s 2> %t.stderr
+// RUN: grep -E "*code should be clang-formatted*" %t.stderr
+

Why not check with `FileCheck`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D69088#1713901 , @Meinersbur wrote:

> In D69088#1713831 , @tyler.nowicki 
> wrote:
>
> > That approach would avoid the inevitable conflicts of having both loop and 
> > transform pragmas on the same loop.
>
>
> I fear it will give us far worse ambiguities. Consider:
>
>   #pragma clang loop unroll(enable) respect_order unrollandjam(enable) 
> unroll_count(4) 
>
>
> How often does it unroll?


Just do not allow this form with respect_order clause.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/test/Analysis/dump_egraph.c:32
+// CHECK-SAME:\"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:\"line\": 15, \"column\": 12, \"file\": \"
+// CHECK-SAME:\}, \"stmt_point_kind\": \"PreStmtPurgeDeadSymbols\",

The actual filename is gracefully omitted so that not to confuse buildbots 
because it's going to be an absolute path.


The joined nodes now actually have the same state. That was intended from the 
start but the original implementation turned out to be buggy.

Before:
F10279509: Screen Shot 2019-10-15 at 7.26.37 PM.png 


After:
F10303986: Screen Shot 2019-10-17 at 4.41.58 PM.png 



Repository:
  rC Clang

https://reviews.llvm.org/D69150

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.c


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -18,10 +18,30 @@
   return *x + *y;
 }
 
-// CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"tag\": null, \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0 
\}\l],\l\"program_state\": null
-
-// CHECK: \"program_points\": [\l\{ \"kind\": 
\"BlockEntrance\", \"block_id\": 1
-
+// CHECK: \"program_points\": [\l
+// CHECK-SAME: \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1,
+// CHECK-SAME:\"terminator\": null, \"term_kind\": null, \"tag\": null,
+// CHECK-SAME:\"node_id\": 1, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"BlockEntrance\", \"block_id\": 1, \"tag\": null,
+// CHECK-SAME:\"node_id\": 2, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"Statement\", \"stmt_kind\": \"IntegerLiteral\",
+// CHECK-SAME:\"stmt_id\": 597, \"pointer\": \"0x{{[0-9a-f]*}}\",
+// CHECK-SAME:\"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:\"line\": 15, \"column\": 12, \"file\": \"
+// CHECK-SAME:\}, \"stmt_point_kind\": \"PreStmtPurgeDeadSymbols\",
+// CHECK-SAME:\"tag\": \"ExprEngine : Clean Node\", \"node_id\": 3,
+// CHECK-SAME:\"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"Statement\", \"stmt_kind\": \"IntegerLiteral\",
+// CHECK-SAME:\"stmt_id\": 597, \"pointer\": \"0x{{[0-9a-f]*}}\",
+// CHECK-SAME:\"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:\"line\": 15, \"column\": 12, \"file\":
+// CHECK-SAME:\}, \"stmt_point_kind\": \"PostStmt\", \"tag\": null,
+// CHECK-SAME:\"node_id\": 4, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \}
+// CHECK-SAME: ]
 
 // CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, 
\"file\": \"{{(.+)}}dump_egraph.c\" \}
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3029,22 +3029,16 @@
   llvm::function_ref PreCallback,
   llvm::function_ref PostCallback,
   llvm::function_ref Stop) {
-const ExplodedNode *FirstHiddenNode = N;
-while (FirstHiddenNode->pred_size() == 1 &&
-   isNodeHidden(*FirstHiddenNode->pred_begin())) {
-  FirstHiddenNode = *FirstHiddenNode->pred_begin();
-}
-const ExplodedNode *OtherNode = FirstHiddenNode;
 while (true) {
-  PreCallback(OtherNode);
-  if (Stop(OtherNode))
+  PreCallback(N);
+  if (Stop(N))
 return true;
 
-  if (OtherNode == N)
+  if (N->succ_size() != 1 || !isNodeHidden(N->getFirstSucc()))
 break;
-  PostCallback(OtherNode);
+  PostCallback(N);
 
-  OtherNode = *OtherNode->succ_begin();
+  N = N->getFirstSucc();
 }
 return false;
   }


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -18,10 +18,30 @@
   return *x + *y;
 }
 
-// CHECK: \"program_points\": [\l\{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null, \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0 \}\l],\l\"program_state\": null
-
-// CHECK: \"program_points\": [\l\{ \"kind\": \"BlockEntrance\", \"block_id\": 1
-
+// CHECK: \"program_points\": [\l
+// CHECK-SAME: \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1,
+// CHECK-SAME:\"terminator\": null, 

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/test/Analysis/dump_egraph.c:32
+// CHECK-SAME:\"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:\"line\": 15, \"column\": 12, \"file\": \"
+// CHECK-SAME:\}, \"stmt_point_kind\": \"PreStmtPurgeDeadSymbols\",

The actual filename is gracefully omitted so that not to confuse buildbots 
because it's going to be an absolute path.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69150



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713831 , @tyler.nowicki 
wrote:

> That approach would avoid the inevitable conflicts of having both loop and 
> transform pragmas on the same loop.


I fear it will give us far worse ambiguities. Consider:

  #pragma clang loop unroll(enable) respect_order unrollandjam(enable) 
unroll_count(4) 

How often does it unroll?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2019-10-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I think this looks pretty good, thanks! I really only noticed style nits. I 
think with some small fixes, we should go ahead and merge it. If you want, I 
can commit it on your behalf, but I know there are other folks at Microsoft 
with commit access to LLVM, if you'd prefer. When you upload the next diff, 
make sure to use the merge point with origin/master as the base, so that the 
patch will apply cleanly.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5959-5960
   CmdArgs.push_back("-cfguard");
-else if (NoChecks)
-  //Emit only the table of address-taken functions.
+}
+else if (GuardArgs.equals_lower("cf,nochecks")) {
+  // Emit only the table of address-taken functions.

Style nit: LLVM puts the close bracket on the same line as else pretty 
consistently. clang-format will make it so.



Comment at: llvm/lib/CodeGen/CFGuardLongjmp.cpp:101
+  // targets.
+  for (MachineInstr *Setjmp : SetjmpCalls) {
+MachineBasicBlock *OldMBB = Setjmp->getParent();

ajpaverd wrote:
> rnk wrote:
> > Rather than changing the machine CFG, I would suggest using 
> > MachineInstr::setPostInstrSymbol, which I've been planning to use for 
> > exactly this purpose. :) You can make labels with MCContext::createSymbol, 
> > and you'll want to come up with symbols that won't clash with C or C++ 
> > symbols. I see MSVC makes labels like `$LN4`, so maybe `$cfgsjN` or 
> > something like that. I think MCContext will add numbers for you.
> Thanks for the suggestion! It looks like MCContext::createSymbol is private, 
> so I'm generating a new symbol name and number for 
> MCContext::getOrCreateSymbol. Does this look OK? 
I guess I was thinking that createTempSymbol would work, but I see you are 
taking steps to make these labels public, so that one is not appropriate. If 
the name matters, then yes, I think getOrCreateSymbol is the right API.



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:220
+GuardDispatch =
+B.CreateCall(GuardDispatchType, GuardDispatchLoad, GuardDispatchArgs);
+  } else if (InvokeInst::classof(CB)) {

rnk wrote:
> You'd want to set the tail call kind here to forward `tail` or `musttail`. 
> This matters for things like member pointer function thunks. This C++ makes a 
> musttail thunk, for example:
>   struct A { virtual void f(); };
>   auto mp = ::f;
> Which gives this IR:
>   define linkonce_odr void @"??_9A@@$BA@AA"(%struct.A* %this, ...) #0 comdat 
> align 2 {
>   entry:
> %0 = bitcast %struct.A* %this to void (%struct.A*, ...)***
> %vtable = load void (%struct.A*, ...)**, void (%struct.A*, ...)*** %0, 
> align 8, !tbaa !3
> %1 = load void (%struct.A*, ...)*, void (%struct.A*, ...)** %vtable, 
> align 8
> musttail call void (%struct.A*, ...) %1(%struct.A* %this, ...)
> ret void
>   }
> This does an indirect virtual call through the 0th slot, and if we lose 
> musttail, it won't perfectly forward arguments.
> 
Thanks, I see the test case for this.



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:254
+  // register (i.e. RAX on 64-bit X86 targets)
+  GuardDispatch->setCallingConv(CallingConv::CFGuard_Dispatch);
+

ajpaverd wrote:
> rnk wrote:
> > So, this isn't going to work if the original calling convention was 
> > something other than the default. For x64, the only commonly used 
> > non-standard convention would be vectorcall. Changing the convention here 
> > in the IR is going to make it hard to pass that along.
> > 
> > I think as you can see from how much code you have here already, replacing 
> > call instructions in general is really hard. These days there are also 
> > bundles, which I guess is something else missing from the above.
> > 
> > Here's a sketch of an alternative approach:
> > - load __guard_dispatch_icall_fptr as normal
> > - get the pre-existing function type of the callee
> > - cast the loaded pointer to the original function pointer type
> > - use `CB->setCalledOperand` to replace the call target
> > - add the original call target as an "argument bundle" to the existing 
> > instruction
> > - change SelectionDAGBuilder.cpp to recognize the new bundle kind and 
> > create a new ArgListEntry in the argument list
> > - make and set a new bit in ArgListEntry, something like isCFGTarget
> > - change CC_X86_Win64_C to put CFGTarget arguments in RAX, similar to how 
> > CCIfNest puts things in R10
> > 
> > This way, the original call instruction remains with exactly the same 
> > arrangement of args, attributes, callbr successors, tail call kind, etc. 
> > All you're doing is changing the callee, and passing the original callee as 
> > an extra argument on the side. Basically, this approach leverages bundles 
> > to avoid messing with the function type, which more or less requires 
> > replacing the call. :)
> > 
> > There are probably issues with this design 

[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713831 , @tyler.nowicki 
wrote:

> In D69088#1713648 , @Meinersbur 
> wrote:
>
> > In D69088#1713623 , @hsaito wrote:
> >
> > > @Meinersbur, if I remember correctly, there was an RFC discussion on this 
> > > topic, right? If yes, would you post the pointer to that? I need a 
> > > refresher on what has been discussed/settled in the past.
> >
> >
> > https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html
>
>
> Sorry if this is answered in the patches but what happens if a loop has both 
> #pragma clang loop and transform defined before it? I guess it probably 
> shouldn't work.


Yes, the plan was to make it an hard error. I unfortunately forgot to add that 
to D69091 , thanks for reminding me. In the 
current implementation the #pragma clang loop would be applied first, but it's 
not intentional.

> Perhaps instead you could create a new option to indicate that the order 
> should be respected.
> 
> #pragma clang loop respect_order  <- optionally with (true) or (false)
> 
> That approach would avoid the inevitable conflicts of having both loop and 
> transform pragmas on the same loop.

There is also a syntax difference:

  #pragma clang loop vectorize_width(4)

compared to

  #pragma clang transform vectorize width(4)

which in a similar form is also how it is done in OpenMP:

  #pragma omp simd simdlen(4)

IMHO, a `respect_order` option is problematic, not only because it influences 
parsing while being parsed. It also makes the preferable behavior clunkier to 
use.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


r375189 - [analyzer] exploded-graph-rewriter: Fix typo in r375186. Unbreaks tests.

2019-10-17 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Oct 17 16:27:35 2019
New Revision: 375189

URL: http://llvm.org/viewvc/llvm-project?rev=375189=rev
Log:
[analyzer] exploded-graph-rewriter: Fix typo in r375186. Unbreaks tests.

Modified:
cfe/trunk/test/Analysis/dump_egraph.c

Modified: cfe/trunk/test/Analysis/dump_egraph.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dump_egraph.c?rev=375189=375188=375189=diff
==
--- cfe/trunk/test/Analysis/dump_egraph.c (original)
+++ cfe/trunk/test/Analysis/dump_egraph.c Thu Oct 17 16:27:35 2019
@@ -18,7 +18,7 @@ int foo() {
   return *x + *y;
 }
 
-// CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"tag\": null, \"node_id\": 1, \"is_sink\":0, \"has_report\": 0 
\}\l],\l\"program_state\": null
+// CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"tag\": null, \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0 
\}\l],\l\"program_state\": null
 
 // CHECK: \"program_points\": [\l\{ \"kind\": 
\"BlockEntrance\", \"block_id\": 1
 


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


[PATCH] D69122: Add support to find out resource dir and add it as compilation args

2019-10-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

@kousikk could we please add support for generic `-extra-arg`, which can then 
add `-resource-dir` like the other clang tools?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69122



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


[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ closed this revision.
NoQ added a comment.

Closed by rC375186  but i forgot the 
phabricator link :)


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

https://reviews.llvm.org/D69015



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Tyler Nowicki via Phabricator via cfe-commits
tyler.nowicki added a comment.

In D69088#1713648 , @Meinersbur wrote:

> In D69088#1713623 , @hsaito wrote:
>
> > @Meinersbur, if I remember correctly, there was an RFC discussion on this 
> > topic, right? If yes, would you post the pointer to that? I need a 
> > refresher on what has been discussed/settled in the past.
>
>
> https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html


Sorry if this is answered in the patches but what happens if a loop has both 
#pragma clang loop and transform defined before it? I guess it probably 
shouldn't work.

Perhaps instead you could create a new option to indicate that the order should 
be respected.

#pragma clang loop respect_order  <- optionally with (true) or (false)

That approach would avoid the inevitable conflicts of having both loop and 
transform pragmas on the same loop.

(Sorry if you received this twice)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


r375186 - [analyzer] Assign truly stable identifiers to exploded nodes.

2019-10-17 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Oct 17 16:10:09 2019
New Revision: 375186

URL: http://llvm.org/viewvc/llvm-project?rev=375186=rev
Log:
[analyzer] Assign truly stable identifiers to exploded nodes.

ExplodedGraph nodes will now have a numeric identifier stored in them
which will keep track of the order in which the nodes were created
and it will be fully deterministic both accross runs and across machines.

This is extremely useful for debugging as it allows reliably setting
conditional breakpoints by node IDs.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/dump_egraph.c
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/edge.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/topology.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=375186=375185=375186=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
Thu Oct 17 16:10:09 2019
@@ -131,10 +131,12 @@ class ExplodedNode : public llvm::Foldin
   /// Succs - The successors of this node.
   NodeGroup Succs;
 
+  int64_t Id;
+
 public:
   explicit ExplodedNode(const ProgramPoint , ProgramStateRef state,
-bool IsSink)
-  : Location(loc), State(std::move(state)), Succs(IsSink) {
+int64_t Id, bool IsSink)
+  : Location(loc), State(std::move(state)), Succs(IsSink), Id(Id) {
 assert(isSink() == IsSink);
   }
 
@@ -258,7 +260,7 @@ public:
   }
   const_succ_range succs() const { return {Succs.begin(), Succs.end()}; }
 
-  int64_t getID(ExplodedGraph *G) const;
+  int64_t getID() const { return Id; }
 
   /// The node is trivial if it has only one successor, only one predecessor,
   /// it's predecessor has only one successor,
@@ -324,7 +326,7 @@ protected:
   BumpVectorContext BVC;
 
   /// NumNodes - The number of nodes in the graph.
-  unsigned NumNodes = 0;
+  int64_t NumNodes = 0;
 
   /// A list of recently allocated nodes that can potentially be recycled.
   NodeVector ChangedNodes;
@@ -358,6 +360,7 @@ public:
   ///  ExplodedGraph for further processing.
   ExplodedNode *createUncachedNode(const ProgramPoint ,
 ProgramStateRef State,
+int64_t Id,
 bool IsSink = false);
 
   std::unique_ptr MakeEmptyGraph() const {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=375186=375185=375186=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Oct 17 16:10:09 2019
@@ -2566,7 +2566,8 @@ BugPathInfo *BugPathGetter::getNextBugPa
 // Create the equivalent node in the new graph with the same state
 // and location.
 ExplodedNode *NewN = GNew->createUncachedNode(
-OrigN->getLocation(), OrigN->getState(), OrigN->isSink());
+OrigN->getLocation(), OrigN->getState(),
+OrigN->getID(), OrigN->isSink());
 
 // Link up the new node with the previous node.
 if (Succ)

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp?rev=375186=375185=375186=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp Thu Oct 17 16:10:09 2019
@@ -283,10 +283,6 @@ ExplodedNode * const *ExplodedNode::Node
   return Storage.getAddrOfPtr1() + 1;
 }
 
-int64_t ExplodedNode::getID(ExplodedGraph 

r375184 - [analyzer] exploded-graph-rewriter: Make node headers a bit lighter.

2019-10-17 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Oct 17 16:10:02 2019
New Revision: 375184

URL: http://llvm.org/viewvc/llvm-project?rev=375184=rev
Log:
[analyzer] exploded-graph-rewriter: Make node headers a bit lighter.

The 50% grey color is too dark on some monitors.

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot?rev=375184=375183=375184=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot Thu Oct 17 
16:10:02 2019
@@ -13,7 +13,7 @@
 // LIGHT: Node0x1 [shape=record,label=<
 // DARK:  Node0x1 [shape=record,color="white",fontcolor="gray80",label=<
 // CHECK-SAME:   
-// LIGHT-SAME: 
+// LIGHT-SAME: 
 // DARK-SAME:  
 // CHECK-SAME:   Node 1 (0x1) - State Unspecified
 // CHECK-SAME: 

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=375184=375183=375184=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Thu Oct 17 16:10:02 2019
@@ -784,7 +784,7 @@ class DotDumpVisitor(object):
 
 self._dump('Node %d (%s) - '
'State %s'
-   % ("gray20" if self._dark_mode else "gray",
+   % ("gray20" if self._dark_mode else "gray70",
   node.node_id, node.ptr, node.state.state_id
   if node.state is not None else 'Unspecified'))
 if node.has_report:


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


r375185 - [analyzer] Display cast kinds in program point dumps.

2019-10-17 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Oct 17 16:10:05 2019
New Revision: 375185

URL: http://llvm.org/viewvc/llvm-project?rev=375185=rev
Log:
[analyzer] Display cast kinds in program point dumps.

Because cast expressions have their own hierarchy, it's extremely useful
to have some information about what kind of casts are we dealing with.

Modified:
cfe/trunk/lib/Analysis/ProgramPoint.cpp
cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=375185=375184=375185=diff
==
--- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
+++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Thu Oct 17 16:10:05 2019
@@ -188,7 +188,11 @@ void ProgramPoint::printJson(llvm::raw_o
 
 Out << "Statement\", \"stmt_kind\": \"" << S->getStmtClassName()
 << "\", \"stmt_id\": " << S->getID(Context)
-<< ", \"pointer\": \"" << (const void *)S << "\", \"pretty\": ";
+<< ", \"pointer\": \"" << (const void *)S << "\", ";
+if (const auto *CS = dyn_cast(S))
+  Out << "\"cast_kind\": \"" << CS->getCastKindName() << "\", ";
+
+Out << "\"pretty\": ";
 
 S->printJson(Out, nullptr, PP, AddQuotes);
 

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot?rev=375185=375184=375185=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot 
(original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot Thu Oct 
17 16:10:05 2019
@@ -116,3 +116,51 @@ Node0x3 [shape=record,label=
   }
 ]}
 \l}"];
+
+// CHECK-NEXT: Program point:
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   main.cpp:8:9:
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: ImplicitCastExpr (LValueToRValue)
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME: S5
+// CHECK-SAME: 
+// CHECK-SAME:   PreStmt
+// CHECK-SAME: 
+// CHECK-SAME: y
+// CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   Tag: 
+// CHECK-SAME:   ExprEngine : Clean Node
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 
+Node0x4 [shape=record,label=
+ "{
+{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": [
+  {
+"kind": "Statement",
+"stmt_kind": "ImplicitCastExpr",
+"cast_kind": "LValueToRValue",
+"stmt_point_kind": "PreStmt",
+"stmt_id": 5,
+"pointer": "0x6",
+"pretty": "y",
+"location": {
+  "file": "main.cpp",
+  "line": 8,
+  "column": 9
+},
+"tag": "ExprEngine : Clean Node"
+  }
+]}
+\l}"];

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=375185=375184=375185=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Thu Oct 17 16:10:05 2019
@@ -73,6 +73,8 @@ class ProgramPoint(object):
 elif self.kind == 'Statement':
 logging.debug(json_pp)
 self.stmt_kind = json_pp['stmt_kind']
+self.cast_kind = json_pp['cast_kind'] \
+if 'cast_kind' in json_pp else None
 self.stmt_point_kind = json_pp['stmt_point_kind']
 self.stmt_id = json_pp['stmt_id']
 self.pointer = json_pp['pointer']
@@ -497,7 +499,9 @@ class DotDumpVisitor(object):
'S%s'
'%s'
'%s'
-   % (self._make_sloc(p.loc), color, p.stmt_kind,
+   % (self._make_sloc(p.loc), color,
+  '%s (%s)' % (p.stmt_kind, p.cast_kind)
+  if p.cast_kind is not None else p.stmt_kind,
   p.stmt_id, stmt_color, p.stmt_point_kind,
   self._short_pretty(p.pretty)
   if not skip_pretty else ''))


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


[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/test/Analysis/exploded-graph-rewriter/node_labels.dot:46
+// COLOR-SAME: Sink Node
+// GREY-SAME:  Sink Node
+// CHECK-SAME:   

Charusso wrote:
> `GREY` typo.
Whoops!!


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

https://reviews.llvm.org/D69015



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


[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

2019-10-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 225540.
NoQ marked an inline comment as done.
NoQ added a comment.

Fxd.


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

https://reviews.llvm.org/D69015

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.c
  clang/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  clang/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/edge.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/topology.dot
  clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -67,6 +67,9 @@
 super(ProgramPoint, self).__init__()
 self.kind = json_pp['kind']
 self.tag = json_pp['tag']
+self.node_id = json_pp['node_id']
+self.is_sink = bool(json_pp['is_sink'])
+self.has_report = bool(json_pp['has_report'])
 if self.kind == 'Edge':
 self.src_id = json_pp['src_id']
 self.dst_id = json_pp['dst_id']
@@ -309,11 +312,9 @@
 
 def construct(self, node_id, json_node):
 logging.debug('Adding ' + node_id)
-self.node_id = json_node['node_id']
-self.ptr = json_node['pointer']
-self.has_report = json_node['has_report']
-self.is_sink = json_node['is_sink']
+self.ptr = node_id[4:]
 self.points = [ProgramPoint(p) for p in json_node['program_points']]
+self.node_id = self.points[-1].node_id
 self.state = ProgramState(json_node['state_id'],
   json_node['program_state']) \
 if json_node['program_state'] is not None else None
@@ -488,12 +489,14 @@
 else:
 color = 'forestgreen'
 
+self._dump('%s.' % p.node_id)
+
 if p.kind == 'Statement':
 # This avoids pretty-printing huge statements such as CompoundStmt.
 # Such statements show up only at [Pre|Post]StmtPurgeDeadSymbols
 skip_pretty = 'PurgeDeadSymbols' in p.stmt_point_kind
 stmt_color = 'cyan3'
-self._dump('%s:'
+self._dump('%s:'
''
'%s '
'S%s'
@@ -506,30 +509,41 @@
   self._short_pretty(p.pretty)
   if not skip_pretty else ''))
 elif p.kind == 'Edge':
-self._dump(''
+self._dump(''
''
'%s'
'[B%d] -\\> [B%d]'
% (color, 'BlockEdge', p.src_id, p.dst_id))
 elif p.kind == 'BlockEntrance':
-self._dump(''
+self._dump(''
''
'%s'
'[B%d]'
% (color, p.kind, p.block_id))
 else:
 # TODO: Print more stuff for other kinds of points.
-self._dump(''
+self._dump(''
''
'%s'
% (color, p.kind))
 
 if p.tag is not None:
-self._dump(''
+self._dump(''
''
'Tag:  '
'%s' % p.tag)
 
+if p.has_report:
+self._dump(''
+   ''
+   'Bug Report Attached'
+   '')
+if p.is_sink:
+self._dump(''
+   ''
+   'Sink Node'
+   '')
+
 def visit_environment(self, e, prev_e=None):
 self._dump('')
 
@@ -786,17 +800,10 @@
 self._dump('color="white",fontcolor="gray80",')
 self._dump('label=<')
 
-self._dump('Node %d (%s) - '
-   'State %s'
+self._dump('State %s'
% ("gray20" if self._dark_mode else "gray70",
-  node.node_id, node.ptr, node.state.state_id
+  node.state.state_id
   

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

2019-10-17 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese added a comment.

While adding the documentation I realized that a better name for this option 
would be `-fmodules-strict-context-hash` to make it clear which hash it's 
referring to.


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

https://reviews.llvm.org/D68528



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-10-17 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc created this revision.
poelmanc added reviewers: malcolm.parsons, alexfh, hokein, aaron.ballman.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

readability-redundant-member-init removes redundant / unnecessary member and 
base class initialization. Unfortunately for the specific case of a copy 
constructor's initialization of a base class, gcc at strict warning levels 
warns if "base class is not initialized in the copy constructor of a derived 
class ".

This patch adds an option `IgnoreBaseInCopyConstructors` defaulting to false 
(thus maintaining current behavior by default) to skip the specific case of 
removal of redundant base class initialization in the copy constructor. 
Enabling this option enables the resulting code to continue to compile 
successfully under `gcc -Werror=extra`. New test cases `WithCopyConstructor1` 
and `WithCopyConstructor2` in 
clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp show 
that it removes redundant members even from copy constructors.

I blindly followed examples for adding a new option and would greatly 
appreciate any review/suggestions/feedback. Thank you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
@@ -1,4 +1,8 @@
 // RUN: %check_clang_tidy %s readability-redundant-member-init %t
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-member-init.IgnoreBaseInCopyConstructors, \
+// RUN:   value: 1}] \
+// RUN: }"
 
 struct S {
   S() = default;
@@ -116,6 +120,35 @@
   };
 };
 
+// struct whose inline copy constructor default-initializes its base class
+struct WithCopyConstructor1 : public T {
+  WithCopyConstructor1(const WithCopyConstructor1& other) : T(),
+f(),
+g()
+  {}
+  S f, g;
+};
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'f' is redundant
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'g' is redundant
+// CHECK-FIXES: WithCopyConstructor1(const WithCopyConstructor1& other) : T()
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: {}
+
+// struct whose copy constructor default-initializes its base class
+struct WithCopyConstructor2 : public T {
+  WithCopyConstructor2(const WithCopyConstructor2& other);
+  S a;
+};
+WithCopyConstructor2::WithCopyConstructor2(const WithCopyConstructor2& other)
+  : T(), a()
+{}
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: initializer for member 'a' is redundant
+// CHECK-FIXES: {{^}}  : T() {{$}}
+// CHECK-NEXT: {}
+
 // Initializer not written
 struct NF1 {
   NF1() {}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
@@ -6,7 +6,8 @@
 Finds member initializations that are unnecessary because the same default
 constructor would be called if they were not present.
 
-Example:
+Example
+---
 
 .. code-block:: c++
 
@@ -18,3 +19,14 @@
   private:
 std::string s;
   };
+
+Options
+---
+
+.. option:: IgnoreBaseInCopyConstructors
+
+When non-zero, the check will ignore unnecessary base class initializations
+within copy constructors. Some compilers issue warnings requiring copy
+constructors to explicitly initialize their base classes.
+
+Default is ``0``.  
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,14 @@
 class RedundantMemberInitCheck : public ClangTidyCheck {
 public:
   RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreBaseInCopyConstructors(Options.get("IgnoreBaseInCopyConstructors", 0))
+  {}
+  void 

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

2019-10-17 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments.



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:70
+};
+if (const int RC = llvm::sys::ExecuteAndWait(ClangBinaryPath, 
PrintResourceDirArgs, {}, Redirects)) {
+  auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());

I haven't looked into this in detail but it feels kind of wasteful to start a 
process just to get this value. Can't we just expose it in some of the clang 
libs we already link against?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69122



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


Re: [PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Tyler Nowicki via cfe-commits
Sorry if this is answered in the patches but what happens if a loop has
both #pragma clang loop and transform defined before it? I guess it
probably shouldn't work.

Perhaps instead you could create a new option to indicate that the order
should be respected.

#pragma clang loop respect_order  <- optionally with (true) or (false)

That approach would avoid the inevitable conflicts of having both loop and
transform pragmas on the same loop.


On Thu, Oct 17, 2019 at 5:27 PM Michael Kruse via Phabricator <
revi...@reviews.llvm.org> wrote:

> Meinersbur added a comment.
>
> In D69088#1713623 , @hsaito
> wrote:
>
> > @Meinersbur, if I remember correctly, there was an RFC discussion on
> this topic, right? If yes, would you post the pointer to that? I need a
> refresher on what has been discussed/settled in the past.
>
>
> https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html
>
>
> Repository:
>   rC Clang
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D69088/new/
>
> https://reviews.llvm.org/D69088
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

2019-10-17 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b0e039a7a7d: [ARM] Fix arm_neon.h with 
-flax-vector-conversions=none, part 3 (authored by efriedma).

Changed prior to commit:
  https://reviews.llvm.org/D68838?vs=224496=225534#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68838

Files:
  clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
  clang/utils/TableGen/NeonEmitter.cpp


Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -1442,7 +1442,8 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector() && LocalCK == ClassB) {
+if (CastToType.isVector() &&
+(LocalCK == ClassB || (T.isHalf() && !T.isScalarForMangling( {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 } else if (CastToType.isVector() && LocalCK == ClassI) {
Index: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
===
--- clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
+++ clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-target-feature +fullfp16 -target-feature +v8.2a\
-// RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone -emit-llvm 
-o - %s \
+// RUN: -fallow-half-arguments-and-returns -flax-vector-conversions=none -S 
-disable-O0-optnone -emit-llvm -o - %s \
 // RUN: | opt -S -mem2reg \
 // RUN: | FileCheck %s
 


Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -1442,7 +1442,8 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector() && LocalCK == ClassB) {
+if (CastToType.isVector() &&
+(LocalCK == ClassB || (T.isHalf() && !T.isScalarForMangling( {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 } else if (CastToType.isVector() && LocalCK == ClassI) {
Index: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
===
--- clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
+++ clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon -target-feature +fullfp16 -target-feature +v8.2a\
-// RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone -emit-llvm -o - %s \
+// RUN: -fallow-half-arguments-and-returns -flax-vector-conversions=none -S -disable-O0-optnone -emit-llvm -o - %s \
 // RUN: | opt -S -mem2reg \
 // RUN: | FileCheck %s
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69140: [clang-offload-wrapper][NFC] Use captured name of the entry type in LIT test

2019-10-17 Thread Sergey Dmitriev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6caada4eb465: [clang-offload-wrapper][NFC] Use captured name 
of the entry type in LIT test (authored by sdmitriev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69140

Files:
  clang/test/Driver/clang-offload-wrapper.c


Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -31,7 +31,7 @@
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]
 
-// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x %__tgt_offload_entry] 
zeroinitializer, section "omp_offloading_entries"
+// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x [[ENTTY]]] zeroinitializer, 
section "omp_offloading_entries"
 
 // CHECK-IR: [[BIN:@.+]] = internal unnamed_addr constant [[BINTY:\[[0-9]+ x 
i8\]]] c"Content of device file{{.+}}"
 


Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -31,7 +31,7 @@
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]
 
-// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x %__tgt_offload_entry] zeroinitializer, section "omp_offloading_entries"
+// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x [[ENTTY]]] zeroinitializer, section "omp_offloading_entries"
 
 // CHECK-IR: [[BIN:@.+]] = internal unnamed_addr constant [[BINTY:\[[0-9]+ x i8\]]] c"Content of device file{{.+}}"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

2019-10-17 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

*using implicit modules in a build where compiler flags in different 
invocations aren't homogeneous, or something along those lines.


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

https://reviews.llvm.org/D68528



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


[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

2019-10-17 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

LGTM with one minor change. Can you add an entry in the modules docs for this 
flag and mention that using it can lead to more PCMs in an implicit build?


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

https://reviews.llvm.org/D68528



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


r375179 - [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

2019-10-17 Thread Eli Friedman via cfe-commits
Author: efriedma
Date: Thu Oct 17 14:57:28 2019
New Revision: 375179

URL: http://llvm.org/viewvc/llvm-project?rev=375179=rev
Log:
[ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

It's completely impossible to check that I've actually found all the
issues, due to the use of macros in arm_neon.h, but hopefully this time
it'll take more than a few hours for someone to find another issue.

I have no idea why, but apparently there's a rule that some, but not
all, builtins which should take an fp16 vector actually take an int8
vector as an argument.  Fix this, and add test coverage.

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


Modified:
cfe/trunk/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
cfe/trunk/utils/TableGen/NeonEmitter.cpp

Modified: cfe/trunk/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c?rev=375179=375178=375179=diff
==
--- cfe/trunk/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c Thu Oct 17 14:57:28 
2019
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-target-feature +fullfp16 -target-feature +v8.2a\
-// RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone -emit-llvm 
-o - %s \
+// RUN: -fallow-half-arguments-and-returns -flax-vector-conversions=none -S 
-disable-O0-optnone -emit-llvm -o - %s \
 // RUN: | opt -S -mem2reg \
 // RUN: | FileCheck %s
 

Modified: cfe/trunk/utils/TableGen/NeonEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/NeonEmitter.cpp?rev=375179=375178=375179=diff
==
--- cfe/trunk/utils/TableGen/NeonEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/NeonEmitter.cpp Thu Oct 17 14:57:28 2019
@@ -1442,7 +1442,8 @@ void Intrinsic::emitBodyAsBuiltinCall()
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector() && LocalCK == ClassB) {
+if (CastToType.isVector() &&
+(LocalCK == ClassB || (T.isHalf() && !T.isScalarForMangling( {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 } else if (CastToType.isVector() && LocalCK == ClassI) {


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


r375177 - [clang-offload-wrapper][NFC] Use captured name of the entry type in LIT test

2019-10-17 Thread Sergey Dmitriev via cfe-commits
Author: sdmitriev
Date: Thu Oct 17 14:55:39 2019
New Revision: 375177

URL: http://llvm.org/viewvc/llvm-project?rev=375177=rev
Log:
[clang-offload-wrapper][NFC] Use captured name of the entry type in LIT test

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

Modified:
cfe/trunk/test/Driver/clang-offload-wrapper.c

Modified: cfe/trunk/test/Driver/clang-offload-wrapper.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-wrapper.c?rev=375177=375176=375177=diff
==
--- cfe/trunk/test/Driver/clang-offload-wrapper.c (original)
+++ cfe/trunk/test/Driver/clang-offload-wrapper.c Thu Oct 17 14:55:39 2019
@@ -31,7 +31,7 @@
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]
 
-// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x %__tgt_offload_entry] 
zeroinitializer, section "omp_offloading_entries"
+// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x [[ENTTY]]] zeroinitializer, 
section "omp_offloading_entries"
 
 // CHECK-IR: [[BIN:@.+]] = internal unnamed_addr constant [[BINTY:\[[0-9]+ x 
i8\]]] c"Content of device file{{.+}}"
 


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


[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

2019-10-17 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc updated this revision to Diff 225523.
poelmanc edited the summary of this revision.
poelmanc added a comment.

In D68682#1707764 , @alexfh wrote:

> cleanupAroundReplacements is not used by clang-format itself. I believe, it's 
> a part of clang-format only because it was easier to implement it on top of 
> some infrastructure clang-format provides.


Thanks for the correction @alexfh. I've updated the patch to call the line 
removal from cleanupAroundReplacements().


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-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-control-flow.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
  clang/include/clang/AST/CommentLexer.h
  clang/include/clang/AST/CommentParser.h
  clang/include/clang/Format/Format.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/Format/Format.cpp

Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -25,6 +25,9 @@
 #include "UnwrappedLineParser.h"
 #include "UsingDeclarationsSorter.h"
 #include "WhitespaceManager.h"
+#include "clang/AST/CommentLexer.h"
+#include "clang/AST/CommentParser.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
@@ -2266,6 +2269,85 @@
 
 } // anonymous namespace
 
+llvm::Expected
+removeNewlyBlankLines(StringRef Code, const tooling::Replacements ) {
+  tooling::Replacements NewReplaces(Replaces);
+  // pair< LineStartPos, CheckedThroughPos > of lines that have been checked
+  // and confirmed that the replacement result so far will be entirely blank.
+  std::list> PotentialWholeLineCuts;
+  int LineStartPos = -1;
+  int LineCheckedThroughPos = -1;
+  bool LineBlankSoFar = true;
+  const char *FileText = Code.data();
+  StringRef FilePath; // Must be the same for every Replacement
+  for (const auto  : Replaces) {
+assert(FilePath.empty() || FilePath == R.getFilePath());
+FilePath = R.getFilePath();
+const int RStartPos = R.getOffset();
+
+int CurrentRLineStartPos = RStartPos;
+while (CurrentRLineStartPos > 0 &&
+   !isVerticalWhitespace(FileText[CurrentRLineStartPos - 1])) {
+  --CurrentRLineStartPos;
+}
+
+assert(CurrentRLineStartPos >= LineStartPos);
+if (CurrentRLineStartPos != LineStartPos) {
+  // We've moved on to a new line. Wrap up the old one before moving on.
+  if (LineBlankSoFar) {
+PotentialWholeLineCuts.push_back(
+std::make_pair(LineStartPos, LineCheckedThroughPos));
+  }
+  LineCheckedThroughPos = CurrentRLineStartPos;
+  LineStartPos = CurrentRLineStartPos;
+  LineBlankSoFar = true;
+}
+
+// Check to see if line from LineCheckedThroughPos to here is blank.
+assert(RStartPos >= LineCheckedThroughPos);
+StringRef PriorTextToCheck(FileText + LineCheckedThroughPos,
+   RStartPos - LineCheckedThroughPos);
+StringRef ReplacementText = R.getReplacementText();
+LineBlankSoFar = LineBlankSoFar && isWhitespace(PriorTextToCheck) &&
+ ReplacementText.empty();
+LineCheckedThroughPos = R.getOffset() + R.getLength();
+  }
+
+  if (LineBlankSoFar) {
+PotentialWholeLineCuts.push_back(
+std::make_pair(LineStartPos, LineCheckedThroughPos));
+  }
+
+  // Now remove whole line if and only if (a) rest of line is blank, and
+  // (b) the original line was *not* blank.
+  for (const auto  : PotentialWholeLineCuts) {
+const int LineStartPos = LineCheckedThrough.first;
+const int CheckedThroughPos = LineCheckedThrough.second;
+
+int LineEndPos = CheckedThroughPos;
+while (LineEndPos < Code.size() &&
+   !isVerticalWhitespace(FileText[LineEndPos])) {
+  ++LineEndPos;
+}
+
+assert(LineEndPos >= CheckedThroughPos);
+StringRef TrailingText(FileText + CheckedThroughPos,
+   LineEndPos - CheckedThroughPos);
+assert(LineEndPos >= LineStartPos);
+StringRef OriginalLine(FileText + LineStartPos, LineEndPos - LineStartPos);
+if (isWhitespace(TrailingText) && !isWhitespace(OriginalLine)) {
+  const char *CutTo = skipNewline(FileText + LineEndPos, Code.end());
+  int CutCount = CutTo - FileText - LineStartPos;
+  llvm::Error Err = NewReplaces.add(
+  tooling::Replacement(FilePath, LineStartPos, CutCount, ""));
+  if (Err) {
+return llvm::Expected(std::move(Err));
+  }
+}
+  }
+  return NewReplaces;
+}
+
 llvm::Expected
 cleanupAroundReplacements(StringRef Code, const 

[PATCH] D69140: [clang-offload-wrapper][NFC] Use captured name of the entry type in LIT test

2019-10-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69140



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


[PATCH] D69144: [Format] Add format check for throwing negative numbers

2019-10-17 Thread Jonathan Thomas via Phabricator via cfe-commits
jonathoma created this revision.
jonathoma added a reviewer: modocache.
jonathoma added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
jonathoma edited the summary of this revision.
jonathoma added reviewers: sammccall, Quuxplusone.

The code `throw -1;` is currently formatted by clang-format as 
`throw - 1;`. This diff adds a fix for this edge case and a test to check
for this in the future.

For context, I am looking into a related bug in the clang-formatting of
coroutine keywords: `co_yield -1;` is also reformatted in this manner 
as `co_yield - 1;`. A later diff will add these changes and tests for the
`co_yield` and `co_return` keywords.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69144

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6912,6 +6912,7 @@
   verifyFormat("alignof(char);", getGoogleStyle());
 
   verifyFormat("return -1;");
+  verifyFormat("throw -1;");
   verifyFormat("switch (a) {\n"
"case -1:\n"
"  break;\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1756,7 +1756,7 @@
 // Use heuristics to recognize unary operators.
 if (PrevToken->isOneOf(tok::equal, tok::l_paren, tok::comma, tok::l_square,
tok::question, tok::colon, tok::kw_return,
-   tok::kw_case, tok::at, tok::l_brace))
+   tok::kw_case, tok::at, tok::l_brace, tok::kw_throw))
   return TT_UnaryOperator;
 
 // There can't be two consecutive binary operators.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6912,6 +6912,7 @@
   verifyFormat("alignof(char);", getGoogleStyle());
 
   verifyFormat("return -1;");
+  verifyFormat("throw -1;");
   verifyFormat("switch (a) {\n"
"case -1:\n"
"  break;\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1756,7 +1756,7 @@
 // Use heuristics to recognize unary operators.
 if (PrevToken->isOneOf(tok::equal, tok::l_paren, tok::comma, tok::l_square,
tok::question, tok::colon, tok::kw_return,
-   tok::kw_case, tok::at, tok::l_brace))
+   tok::kw_case, tok::at, tok::l_brace, tok::kw_throw))
   return TT_UnaryOperator;
 
 // There can't be two consecutive binary operators.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69140: [clang-offload-wrapper][NFC] Use captured name of the entry type in LIT test

2019-10-17 Thread Sergey Dmitriev via Phabricator via cfe-commits
sdmitriev created this revision.
sdmitriev added a reviewer: ABataev.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69140

Files:
  clang/test/Driver/clang-offload-wrapper.c


Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -31,7 +31,7 @@
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]
 
-// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x %__tgt_offload_entry] 
zeroinitializer, section "omp_offloading_entries"
+// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x [[ENTTY]]] zeroinitializer, 
section "omp_offloading_entries"
 
 // CHECK-IR: [[BIN:@.+]] = internal unnamed_addr constant [[BINTY:\[[0-9]+ x 
i8\]]] c"Content of device file{{.+}}"
 


Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -31,7 +31,7 @@
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]
 
-// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x %__tgt_offload_entry] zeroinitializer, section "omp_offloading_entries"
+// CHECK-IR: [[DUMMY:@.+]] = hidden constant [0 x [[ENTTY]]] zeroinitializer, section "omp_offloading_entries"
 
 // CHECK-IR: [[BIN:@.+]] = internal unnamed_addr constant [[BINTY:\[[0-9]+ x i8\]]] c"Content of device file{{.+}}"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-17 Thread Steven Wan via Phabricator via cfe-commits
stevewan updated this revision to Diff 225521.
stevewan added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/Driver/Inputs/aix_ppc_tree/powerpc-ibm-aix7.1.0.0/dummy.a
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crt0_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crti_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/gcrt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/gcrt0_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/mcrt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/mcrt0_64.o
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- /dev/null
+++ clang/test/Driver/aix-ld.c
@@ -0,0 +1,177 @@
+// General tests that ld invocations on AIX targets are sane. Note that we use
+// sysroot to make these tests independent of the host system.
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32 %s
+// CHECK-LD32-NOT: warning:
+// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-NOT: "-bnso"
+// CHECK-LD32: "-b32" 
+// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000" 
+// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32: "-L[[SYSROOT]]/usr/lib" 
+// CHECK-LD32: "-lc"
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64 %s
+// CHECK-LD64-NOT: warning:
+// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64-NOT: "-bnso"
+// CHECK-LD64: "-b64" 
+// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000" 
+// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64: "-L[[SYSROOT]]/usr/lib" 
+// CHECK-LD64: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -pthread \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-PTHREAD %s
+// CHECK-LD32-PTHREAD-NOT: warning:
+// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PTHREAD-NOT: "-bnso"
+// CHECK-LD32-PTHREAD: "-b32" 
+// CHECK-LD32-PTHREAD: "-bpT:0x1000" "-bpD:0x2000" 
+// CHECK-LD32-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -pthreads \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-PTHREAD %s
+// CHECK-LD64-PTHREAD-NOT: warning:
+// CHECK-LD64-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64-PTHREAD-NOT: "-bnso"
+// CHECK-LD64-PTHREAD: "-b64" 
+// CHECK-LD64-PTHREAD: "-bpT:0x1" "-bpD:0x11000" 
+// CHECK-LD64-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64-PTHREAD: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64-PTHREAD: "-lpthreads"
+// CHECK-LD64-PTHREAD: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable profiling.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -p \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-PROF %s
+// CHECK-LD32-PROF-NOT: warning:
+// CHECK-LD32-PROF: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PROF: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PROF-NOT: "-bnso"
+// CHECK-LD32-PROF: "-b32" 
+// CHECK-LD32-PROF: "-bpT:0x1000" "-bpD:0x2000" 
+// 

[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713623 , @hsaito wrote:

> @Meinersbur, if I remember correctly, there was an RFC discussion on this 
> topic, right? If yes, would you post the pointer to that? I need a refresher 
> on what has been discussed/settled in the past.


https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In D69088#1713147 , @ABataev wrote:

> Why not try to improve the existing #pragma clang loop rather than add a new 
> pragma with almost the same behavior?


The behavior and syntax is different. #pragma clang loop ignores the order, i.e.

  #pragma clang loop unroll(enable)
  #pragma clang loop distribute(enable)

and

  #pragma clang loop distribute(enable)
  #pragma clang loop unroll(enable)

and

  #pragma clang loop unroll(enable) distribute(enable)

are the same. Changing that would be a breaking change.

Syntactically, every option is it's own transformation, e.g.

  #pragma clang loop unroll(enable) distribute(enable) unroll_count(2)

could be interpreted as 3 transformations (LoopUnroll even exists twice in the 
pass pipeline). I prefer OpenMP's directive-with-clauses syntax, which we need 
to implement anyway for the OpenMP loop transformations.

In the future, I would also like to add non-loop transformation, such that the 
`loop` namespace l


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Hideki Saito via Phabricator via cfe-commits
hsaito added a comment.

@Meinersbur, if I remember correctly, there was an RFC discussion on this 
topic, right? If yes, would you post the pointer to that? I need a refresher on 
what has been discussed/settled in the past.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


r375167 - [OPENMP]Dow not emit warnings for uninitialized loop counters.

2019-10-17 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Oct 17 13:35:08 2019
New Revision: 375167

URL: http://llvm.org/viewvc/llvm-project?rev=375167=rev
Log:
[OPENMP]Dow not emit warnings for uninitialized loop counters.

In OpenMP constructs all counters are initialized and we should not emit
warnings about uninitialized privatized loop control variables.

Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/Analysis/cfg-openmp.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_linear_messages.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=375167=375166=375167=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Thu Oct 17 13:35:08 2019
@@ -4824,7 +4824,8 @@ CFGBlock *CFGBuilder::VisitOMPExecutable
   }
   // Visit associated structured block if any.
   if (!D->isStandaloneDirective())
-if (Stmt *S = D->getStructuredBlock()) {
+if (CapturedStmt *CS = D->getInnermostCapturedStmt()) {
+  Stmt *S = CS->getCapturedStmt();
   if (!isa(S))
 addLocalScopeAndDtors(S);
   if (CFGBlock *R = addStmt(S))

Modified: cfe/trunk/test/Analysis/cfg-openmp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-openmp.cpp?rev=375167=375166=375167=diff
==
--- cfe/trunk/test/Analysis/cfg-openmp.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-openmp.cpp Thu Oct 17 13:35:08 2019
@@ -27,72 +27,6 @@ void xxx(int argc) {
 // CHECK-NEXT:   [B1.[[#CRIT+3]]];
 #pragma omp critical
   argc = x;
-// CHECK-NEXT:  [[#DPF:]]: x
-// CHECK-NEXT:  [[#DPF+1]]: [B1.[[#DPF]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#DPF+2]]: argc
-// CHECK-NEXT:  [[#DPF+3]]: [B1.[[#DPF+2]]] = [B1.[[#DPF+1]]]
-// CHECK-NEXT:  [[#DPF+4]]: cond
-// CHECK-NEXT:  [[#DPF+5]]: [B1.[[#DPF+4]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#DPF+6]]: [B1.[[#DPF+5]]] (ImplicitCastExpr, 
IntegralToBoolean, _Bool)
-// CHECK-NEXT:  [[#DPF+7]]: fp
-// CHECK-NEXT:  [[#DPF+8]]: rd
-// CHECK-NEXT:  [[#DPF+9]]: #pragma omp distribute parallel for if(parallel: 
cond) firstprivate(fp) reduction(+: rd)
-// CHECK-NEXT:for (int i = 0; i < 10; ++i)
-// CHECK-NEXT:[B1.[[#DPF+3]]];
-#pragma omp distribute parallel for if(parallel:cond) firstprivate(fp) 
reduction(+:rd)
-  for (int i = 0; i < 10; ++i)
-argc = x;
-// CHECK-NEXT:  [[#DPFS:]]: x
-// CHECK-NEXT:  [[#DPFS+1]]: [B1.[[#DPFS]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#DPFS+2]]: argc
-// CHECK-NEXT:  [[#DPFS+3]]: [B1.[[#DPFS+2]]] = [B1.[[#DPFS+1]]]
-// CHECK-NEXT:  [[#DPFS+4]]: cond
-// CHECK-NEXT:  [[#DPFS+5]]: [B1.[[#DPFS+4]]] (ImplicitCastExpr, 
LValueToRValue, int)
-// CHECK-NEXT:  [[#DPFS+6]]: [B1.[[#DPFS+5]]] (ImplicitCastExpr, 
IntegralToBoolean, _Bool)
-// CHECK-NEXT:  [[#DPFS+7]]: fp
-// CHECK-NEXT:  [[#DPFS+8]]: rd
-// CHECK-NEXT:  [[#DPFS+9]]: #pragma omp distribute parallel for simd if(cond) 
firstprivate(fp) reduction(-: rd)
-// CHECK-NEXT:for (int i = 0; i < 10; ++i)
-// CHECK-NEXT:[B1.[[#DPFS+3]]];
-#pragma omp distribute parallel for simd if(cond)  firstprivate(fp) 
reduction(-:rd)
-  for (int i = 0; i < 10; ++i)
-argc = x;
-// CHECK-NEXT:  [[#DS:]]: x
-// CHECK-NEXT:  [[#DS+1]]: [B1.[[#DS]]] (ImplicitCastExpr, LValueToRValue, int)
-// CHECK-NEXT:  [[#DS+2]]: argc
-// CHECK-NEXT:  [[#DS+3]]: [B1.[[#DS+2]]] = [B1.[[#DS+1]]]
-// CHECK-NEXT:  [[#DS+4]]: #pragma omp distribute simd
-// CHECK-NEXT:for (int i = 0; i < 10; ++i)
-// CHECK-NEXT:[B1.[[#DS+3]]];
-#pragma omp distribute simd
-  for (int i = 0; i < 10; ++i)
-argc = x;
-// CHECK-NEXT:  [[#FOR:]]: x
-// CHECK-NEXT:  [[#FOR+1]]: [B1.[[#FOR]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#FOR+2]]: argc
-// CHECK-NEXT:  [[#FOR+3]]: [B1.[[#FOR+2]]] = [B1.[[#FOR+1]]]
-// CHECK-NEXT:  [[#FOR+4]]: lin
-// CHECK-NEXT:  [[#FOR+5]]: step
-// CHECK-NEXT:  [[#FOR+6]]: [B1.[[#FOR+5]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#FOR+7]]: #pragma omp for linear(lin: step)
-// CHECK-NEXT:for (int i = 0; i < 10; ++i)
-// CHECK-NEXT:[B1.[[#FOR+3]]];
-#pragma omp for linear(lin : step)
-  for (int i = 0; i < 10; ++i)
-argc = x;
-// CHECK-NEXT:  [[#FS:]]: x
-// CHECK-NEXT:  [[#FS+1]]: [B1.[[#FS]]] (ImplicitCastExpr, LValueToRValue, int)
-// CHECK-NEXT:  [[#FS+2]]: argc
-// CHECK-NEXT:  [[#FS+3]]: [B1.[[#FS+2]]] = [B1.[[#FS+1]]]
-// CHECK-NEXT:  [[#FS+4]]: lin
-// CHECK-NEXT:  [[#FS+5]]: step
-// CHECK-NEXT:  [[#FS+6]]: [B1.[[#FS+5]]] (ImplicitCastExpr, LValueToRValue, 
int)
-// CHECK-NEXT:  [[#FS+7]]: #pragma omp for simd linear(lin: step)
-// CHECK-NEXT:for (int i = 0; i < 10; ++i)
-// CHECK-NEXT:[B1.[[#FS+3]]];
-#pragma omp for simd linear(lin: step)
-  

[PATCH] D67992: [Sema] Add MacroQualified case for FunctionTypeUnwrapper

2019-10-17 Thread Jung-uk Kim via Phabricator via cfe-commits
jkim added a comment.

Ping...  @rsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67992



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


[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

2019-10-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/SemaOverload.cpp:594
   };
+  struct CNSInfo {
+TemplateArgumentList *TemplateArgs;

Please add a documentation comment.



Comment at: clang/lib/Sema/SemaOverload.cpp:10707
 
-  case Sema::TDK_InstantiationDepth:
+  case Sema::TDK_ConstraintsNotSatisfied:
 return 4;

I think we should probably rank this higher -- maybe at the same level as a 
deduction or substitution failure, or maybe just above that. But I'm happy to 
wait and iterate on that once we have library code to experiment with.



Comment at: clang/lib/Sema/SemaTemplate.cpp:4235
 /*PartialTemplateArgs=*/false, Converted,
 /*UpdateArgsWithConversion=*/false))
 return ExprError();

(Not directly related to this patch, feel free to address separately:) Passing 
false for UpdateArgsWithConversion here seems surprising: shouldn't we be using 
the converted arguments in the satisfaction check and storing the converted 
arguments on the AST?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D41569



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


[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

This is... rather oddly-structured output. My brain refuses to accept that the 
most-indented phase is the input.
Perhaps we should do `llvm::errs().indent(MaxIdent-Ident)`. This should give us 
something like this (withMaxIdent=9), which is somewhat easier to grok, IMO:

  0: input, "/home/michliao/dummy.cpp", cuda, (host-cuda)
   1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
2: compiler, {1}, ir, (host-cuda)
  3: input, "/home/michliao/dummy.cpp", cuda, (device-cuda, sm_30)
   4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
5: compiler, {4}, ir, (device-cuda, sm_30)
 6: backend, {5}, assembler, (device-cuda, sm_30)
  7: assembler, {6}, object, (device-cuda, sm_30)
   8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {7}, object
   9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {6}, assembler
  10: input, "/home/michliao/dummy.cpp", cuda, (device-cuda, sm_60)
   11: preprocessor, {10}, cuda-cpp-output, (device-cuda, sm_60)
12: compiler, {11}, ir, (device-cuda, sm_60)
 13: backend, {12}, assembler, (device-cuda, sm_60)
  14: assembler, {13}, object, (device-cuda, sm_60)
   15: offload, "device-cuda (nvptx64-nvidia-cuda:sm_60)" {14}, object
   16: offload, "device-cuda (nvptx64-nvidia-cuda:sm_60)" {13}, assembler
17: linker, {8, 9, 15, 16}, cuda-fatbin, (device-cuda)
 18: offload, "host-cuda (x86_64-unknown-linux-gnu)" {2}, "device-cuda 
(nvptx64-nvidia-cuda)" {17}, ir
  19: backend, {18}, assembler, (host-cuda)
   20: assembler, {19}, object, (host-cuda)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69124



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


[PATCH] D69129: [AMDGPU] Fix assertion due to initializer list

2019-10-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: rjmccall.
Herald added subscribers: t-tye, tpr, dstuttard, wdng, kzhuravl.

Sometimes a global var is replaced by a different llvm value. clang use 
GetAddrOfGlobalVar to get the original llvm global variable.
For most targets, GetAddrOfGlobalVar returns either the llvm global variable or 
a bitcast of the llvm global variable.
However, for AMDGPU target, GetAddrOfGlobalVar returns the addrspace cast or 
addrspace cast plus bitcast of the llvm global variable.
To get the llvm global variable, these casts need to be stripped, otherwise 
there is assertion.

This patch fixes that.


https://reviews.llvm.org/D69129

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCXX/cxx11-extern-constexpr.cpp

Index: test/CodeGenCXX/cxx11-extern-constexpr.cpp
===
--- test/CodeGenCXX/cxx11-extern-constexpr.cpp
+++ test/CodeGenCXX/cxx11-extern-constexpr.cpp
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX11
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX17
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=X86,CXX11X86
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=X86,CXX17X86
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | FileCheck %s --check-prefixes=AMD,CXX11AMD
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | FileCheck %s --check-prefixes=AMD,CXX17AMD
 
 struct A {
   static const int Foo = 123;
 };
-// CHECK: @_ZN1A3FooE = constant i32 123, align 4
+// X86: @_ZN1A3FooE = constant i32 123, align 4
+// AMD: @_ZN1A3FooE = addrspace(4) constant i32 123, align 4
 const int *p = ::Foo; // emit available_externally
 const int A::Foo;   // convert to full definition
 
@@ -16,7 +19,8 @@
   // Deferred initialization of the structure here requires changing
   // the type of the global variable: the initializer list does not include
   // the tail padding.
-  // CXX11: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } { i32 42, i8 43 },
+  // CXX11X86: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } { i32 42, i8 43 },
+  // CXX11AMD: @_ZN9CreatePOD3podE = available_externally addrspace(1) constant { i32, i8 } { i32 42, i8 43 },
   static constexpr PODWithInit pod{};
 };
 const int *p_pod = ::pod.g;
@@ -30,29 +34,40 @@
 };
 
 struct Foo {
-  // CXX11: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant i32 42,
-  // CXX17: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42,
+  // CXX11X86: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant i32 42,
+  // CXX17X86: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42,
+  // CXX11AMD: @_ZN3Foo21ConstexprStaticMemberE = available_externally addrspace(4) constant i32 42,
+  // CXX17AMD: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr addrspace(4) constant i32 42,
   static constexpr int ConstexprStaticMember = 42;
-  // CHECK: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 43,
+  // X86: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 43,
+  // AMD: @_ZN3Foo17ConstStaticMemberE = available_externally addrspace(4) constant i32 43,
   static const int ConstStaticMember = 43;
 
-  // CXX11: @_ZN3Foo23ConstStaticStructMemberE = available_externally constant %struct.Bar { i32 44 },
-  // CXX17: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant %struct.Bar { i32 44 },
+  // CXX11X86: @_ZN3Foo23ConstStaticStructMemberE = available_externally constant %struct.Bar { i32 44 },
+  // CXX17X86: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant %struct.Bar { i32 44 },
+  // CXX11AMD: @_ZN3Foo23ConstStaticStructMemberE = available_externally addrspace(1) constant %struct.Bar { i32 44 },
+  // CXX17AMD: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr addrspace(1) constant %struct.Bar { i32 44 },
   static constexpr Bar ConstStaticStructMember = {44};
 
-  // CXX11: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global %struct.MutableBar,
-  // CXX17: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr global %struct.MutableBar { i32 45 },
+  // CXX11X86: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global %struct.MutableBar,
+  // CXX17X86: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr global %struct.MutableBar { i32 45 },
+  // CXX11AMD: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external addrspace(1) global %struct.MutableBar,
+  // CXX17AMD: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr addrspace(1) global %struct.MutableBar { i32 45 },
   static constexpr MutableBar ConstexprStaticMutableStructMember = {45};
 };
-// CHECK: 

[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D69124#1713360 , @tra wrote:

> Could you give an example of before/after output?


For HIP

  $ clang -x hip -ccc-print-phases --cuda-gpu-arch=gfx900 
--cuda-gpu-arch=gfx906 -c ~/dummy.cpp 
   0: input, "/home/michliao/dummy.cpp", hip, (host-hip)
  1: preprocessor, {0}, hip-cpp-output, (host-hip)
 2: compiler, {1}, ir, (host-hip)
  3: input, "/home/michliao/dummy.cpp", hip, (device-hip, gfx900)
 4: preprocessor, {3}, hip-cpp-output, (device-hip, gfx900)
5: compiler, {4}, ir, (device-hip, gfx900)
   6: linker, {5}, image, (device-hip, gfx900)
  7: offload, "device-hip (amdgcn-amd-amdhsa:gfx900)" {6}, image
  8: input, "/home/michliao/dummy.cpp", hip, (device-hip, gfx906)
 9: preprocessor, {8}, hip-cpp-output, (device-hip, gfx906)
10: compiler, {9}, ir, (device-hip, gfx906)
   11: linker, {10}, image, (device-hip, gfx906)
  12: offload, "device-hip (amdgcn-amd-amdhsa:gfx906)" {11}, image
 13: linker, {7, 12}, hip-fatbin, (device-hip)
14: offload, "host-hip (x86_64-unknown-linux-gnu)" {2}, "device-hip 
(amdgcn-amd-amdhsa)" {13}, ir
   15: backend, {14}, assembler, (host-hip)
  16: assembler, {15}, object, (host-hip)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69124



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


[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D69124#1713360 , @tra wrote:

> Could you give an example of before/after output?




  $ clang -x cuda -ccc-print-phases --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_60 
-c ~/dummy.cpp 
   0: input, "/home/michliao/dummy.cpp", cuda, (host-cuda)
  1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
 2: compiler, {1}, ir, (host-cuda)
   3: input, "/home/michliao/dummy.cpp", cuda, (device-cuda, sm_30)
  4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
 5: compiler, {4}, ir, (device-cuda, sm_30)
6: backend, {5}, assembler, (device-cuda, sm_30)
   7: assembler, {6}, object, (device-cuda, sm_30)
  8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {7}, object
  9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {6}, assembler
   10: input, "/home/michliao/dummy.cpp", cuda, (device-cuda, sm_60)
  11: preprocessor, {10}, cuda-cpp-output, (device-cuda, sm_60)
 12: compiler, {11}, ir, (device-cuda, sm_60)
13: backend, {12}, assembler, (device-cuda, sm_60)
   14: assembler, {13}, object, (device-cuda, sm_60)
  15: offload, "device-cuda (nvptx64-nvidia-cuda:sm_60)" {14}, object
  16: offload, "device-cuda (nvptx64-nvidia-cuda:sm_60)" {13}, assembler
 17: linker, {8, 9, 15, 16}, cuda-fatbin, (device-cuda)
18: offload, "host-cuda (x86_64-unknown-linux-gnu)" {2}, "device-cuda 
(nvptx64-nvidia-cuda)" {17}, ir
   19: backend, {18}, assembler, (host-cuda)
  20: assembler, {19}, object, (host-cuda)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69124



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


[PATCH] D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output

2019-10-17 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

I think you could've just used `CHECK-DAG` to fix the tests. It *might* be a 
bit more robust. Although just reordering checks seems perfectly fine too.
https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive

Using `std::set` here sounds reasonable to me but I'd like @arphaman to be 
aware of this. BTW should we change `std::vector Dependencies;` in 
`DependencyCollector` to `set` too?

Also, I'm wondering why is the behavior different on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69090



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


[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Could you give an example of before/after output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69124



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


[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: tra, sfantao, echristo.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hliao added a comment.

this patch enables the dumping of actions in the hierarchy or tree. In most 
cases, it's a linear list but, for offload compilation, a tree representation 
is more intuitive. Even though there are cross-subtree edges, they are rare and 
also noted in the corresponding actions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69124

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1806,7 +1806,8 @@
 // and latest-occuring action. Traversal is in pre-order, visiting the
 // inputs to each action before printing the action itself.
 static unsigned PrintActions1(const Compilation , Action *A,
-  std::map ) {
+  std::map ,
+  unsigned Ident = 0) {
   if (Ids.count(A)) // A was already visited.
 return Ids[A];
 
@@ -1818,7 +1819,7 @@
 os << "\"" << IA->getInputArg().getValue() << "\"";
   } else if (BindArchAction *BIA = dyn_cast(A)) {
 os << '"' << BIA->getArchName() << '"' << ", {"
-   << PrintActions1(C, *BIA->input_begin(), Ids) << "}";
+   << PrintActions1(C, *BIA->input_begin(), Ids, Ident + 1) << "}";
   } else if (OffloadAction *OA = dyn_cast(A)) {
 bool IsFirst = true;
 OA->doOnEachDependence(
@@ -1841,7 +1842,7 @@
 os << ":" << BoundArch;
   os << ")";
   os << '"';
-  os << " {" << PrintActions1(C, A, Ids) << "}";
+  os << " {" << PrintActions1(C, A, Ids, Ident + 1) << "}";
   IsFirst = false;
 });
   } else {
@@ -1850,7 +1851,7 @@
 if (AL->size()) {
   const char *Prefix = "{";
   for (Action *PreRequisite : *AL) {
-os << Prefix << PrintActions1(C, PreRequisite, Ids);
+os << Prefix << PrintActions1(C, PreRequisite, Ids, Ident + 1);
 Prefix = ", ";
   }
   os << "}";
@@ -1874,8 +1875,9 @@
 
   unsigned Id = Ids.size();
   Ids[A] = Id;
-  llvm::errs() << Id << ": " << os.str() << ", "
-   << types::getTypeName(A->getType()) << offload_os.str() << "\n";
+  llvm::errs().indent(Ident)
+  << Id << ": " << os.str() << ", " << types::getTypeName(A->getType())
+  << offload_os.str() << "\n";
 
   return Id;
 }


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1806,7 +1806,8 @@
 // and latest-occuring action. Traversal is in pre-order, visiting the
 // inputs to each action before printing the action itself.
 static unsigned PrintActions1(const Compilation , Action *A,
-  std::map ) {
+  std::map ,
+  unsigned Ident = 0) {
   if (Ids.count(A)) // A was already visited.
 return Ids[A];
 
@@ -1818,7 +1819,7 @@
 os << "\"" << IA->getInputArg().getValue() << "\"";
   } else if (BindArchAction *BIA = dyn_cast(A)) {
 os << '"' << BIA->getArchName() << '"' << ", {"
-   << PrintActions1(C, *BIA->input_begin(), Ids) << "}";
+   << PrintActions1(C, *BIA->input_begin(), Ids, Ident + 1) << "}";
   } else if (OffloadAction *OA = dyn_cast(A)) {
 bool IsFirst = true;
 OA->doOnEachDependence(
@@ -1841,7 +1842,7 @@
 os << ":" << BoundArch;
   os << ")";
   os << '"';
-  os << " {" << PrintActions1(C, A, Ids) << "}";
+  os << " {" << PrintActions1(C, A, Ids, Ident + 1) << "}";
   IsFirst = false;
 });
   } else {
@@ -1850,7 +1851,7 @@
 if (AL->size()) {
   const char *Prefix = "{";
   for (Action *PreRequisite : *AL) {
-os << Prefix << PrintActions1(C, PreRequisite, Ids);
+os << Prefix << PrintActions1(C, PreRequisite, Ids, Ident + 1);
 Prefix = ", ";
   }
   os << "}";
@@ -1874,8 +1875,9 @@
 
   unsigned Id = Ids.size();
   Ids[A] = Id;
-  llvm::errs() << Id << ": " << os.str() << ", "
-   << types::getTypeName(A->getType()) << offload_os.str() << "\n";
+  llvm::errs().indent(Ident)
+  << Id << ": " << os.str() << ", " << types::getTypeName(A->getType())
+  << offload_os.str() << "\n";
 
   return Id;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69124: [clang][driver] Print compilation phases with indentation.

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

this patch enables the dumping of actions in the hierarchy or tree. In most 
cases, it's a linear list but, for offload compilation, a tree representation 
is more intuitive. Even though there are cross-subtree edges, they are rare and 
also noted in the corresponding actions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69124



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


[PATCH] D69122: Add support to find out resource dir and add it as compilation args

2019-10-17 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk created this revision.
kousikk added reviewers: arphaman, Bigcheese, jkorous, dexonsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
kousikk edited the summary of this revision.
kousikk edited the summary of this revision.

If -resource-dir is not specified as part of the compilation command, then by 
default
clang-scan-deps picks up a directory relative to its own path as 
resource-directory.
This is probably not the right behavior - since resource directory should be 
picked relative
to the path of the clang-compiler in the compilation command.
This patch adds support for it along with a cache to store the resource-dir 
paths based on
compiler paths.

Notes:

1. "-resource-dir" is a behavior that's specific to clang, gcc does not have 
that flag. That's why if I'm not able to find a resource-dir, I quietly ignore 
it.
2. Should I also use the mtime of the compiler in the cache? I think its not 
strictly necessary since we assume the filesystem is immutable.
3. From my testing, this does not regress performance.
4. Will try to get this tested on Windows.

But basically the problem that this patch is trying to solve is, clients might 
not always want to specify
"-resource-dir" in their compile commands, so scan-deps must auto-infer it 
correctly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69122

Files:
  clang/include/clang/Tooling/ArgumentsAdjusters.h
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/lib/Tooling/CommonOptionsParser.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -12,6 +12,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Options.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
@@ -38,6 +39,64 @@
   raw_ostream 
 };
 
+class ResourceDirectoryCache {
+public:
+  /// FindResourceDir finds the resource directory relative to the clang compiler
+  /// being used in Args, by running it with "-print-resource-dir" option and
+  /// cache the results for reuse.
+  /// \returns resource directory path associated with the given invocation command.
+  std::string FindResourceDir(const tooling::CommandLineArguments , StringRef Directory) {
+if (Args.size() < 1)
+  return "";
+
+const std::string  = MakeAbsolutePath(Directory, Args[0]);
+
+std::unique_lock LockGuard(CacheLock);
+const auto& CachedResourceDir = Cache.find(ClangBinaryPath);
+if (CachedResourceDir != Cache.end())
+  return CachedResourceDir->second;
+
+std::vector PrintResourceDirArgs{"clang", "-print-resource-dir"};
+llvm::SmallString<64> OutputFile, ErrorFile;
+llvm::sys::fs::createTemporaryFile("print-resource-dir-output", "" /*no-suffix*/, OutputFile);
+llvm::sys::fs::createTemporaryFile("print-resource-dir-error", ""/*no-suffix*/, ErrorFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+llvm::FileRemover ErrorRemover(ErrorFile.c_str());
+llvm::Optional Redirects[] = {
+  {""}, //Stdin
+  StringRef(OutputFile),
+  StringRef(ErrorFile),
+};
+if (const int RC = llvm::sys::ExecuteAndWait(ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) {
+  auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());
+  llvm::errs() << ErrorBuf.get()->getBuffer();
+  return "";
+}
+
+auto OutputBuf = llvm::MemoryBuffer::getFile(OutputFile.c_str());
+if (!OutputBuf)
+  return "";
+StringRef Output = OutputBuf.get()->getBuffer().rtrim('\n');
+
+Cache[ClangBinaryPath] = Output.str();
+return Cache[ClangBinaryPath];
+  }
+
+private:
+  /// \returns the absolute path formed by joining the current directory with the given path.
+  std::string MakeAbsolutePath(StringRef CurrentDir, StringRef Path) {
+if (Path.empty())
+  return "";
+llvm::SmallString<256> InitialDirectory(CurrentDir);
+llvm::SmallString<256> AbsolutePath(Path);
+llvm::sys::fs::make_absolute(InitialDirectory, AbsolutePath);
+return AbsolutePath.str();
+  }
+
+  std::map Cache;
+  std::mutex CacheLock;
+};
+
 /// The high-level implementation of the dependency discovery tool that runs on
 /// an individual worker thread.
 class DependencyScanningTool {
@@ -218,12 +277,14 @@
   auto AdjustingCompilations =
   std::make_unique(
   std::move(Compilations));
+  ResourceDirectoryCache ResourceDirCache;
   AdjustingCompilations->appendArgumentsAdjuster(
-  [](const tooling::CommandLineArguments , StringRef FileName) {
+  [](const tooling::CommandLineArguments , StringRef FileName, StringRef Directory) {

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

2019-10-17 Thread Thomas Lively via Phabricator via cfe-commits
tlively reclaimed this revision.
tlively added a comment.

I'm re-opening this revision. After discussion on 
https://github.com/WebAssembly/simd/issues/118, there is clear consensus that 
we do not want to break WebAssembly's abstraction and consider underlying 
platforms, so shuffles should not be merged without some sort of modification 
to the spec to serve as a platform-agnostic indication that the merged shuffle 
will be better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



___
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

2019-10-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

Extra note: an older version of the patch has been tested by the firefox team 
without much performance impact, (and no test failure), see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1588710


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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


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

2019-10-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 225462.
serge-sans-paille added a comment.

Moved the implementation to a specialization of `emitStackProbeInline` that 
used to be Windows-centric. Provide a generic implementation that generates 
inline assembly instead. that way we don't clutter existing functions, and only 
add a new case to `emitSPUpdate`.

Handle large stack allocation with a loop instead of a (large) basic block.

@rnk: this is far less intrusive and more integrated to the existing structure, 
thanks a lot for hinting in that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, 880(%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll

[PATCH] D69088: [Lex] #pragma clang transform

2019-10-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

Why not try to improve the existing #pragma clang loop rather than add a new 
pragma with almost the same behavior?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69088



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


r375134 - [OPENMP]Improve use of the global tid parameter.

2019-10-17 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Oct 17 10:12:03 2019
New Revision: 375134

URL: http://llvm.org/viewvc/llvm-project?rev=375134=rev
Log:
[OPENMP]Improve use of the global tid parameter.

If we can determined, that the global tid parameter can be used in the
function, better to use it rather than calling __kmpc_global_thread_num
function.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/openmp_win_codegen.cpp
cfe/trunk/test/OpenMP/parallel_for_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=375134=375133=375134=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Oct 17 10:12:03 2019
@@ -1697,18 +1697,23 @@ llvm::Value *CGOpenMPRuntime::getThreadI
   return ThreadID;
   }
   // If exceptions are enabled, do not use parameter to avoid possible crash.
-  if (!CGF.EHStack.requiresLandingPad() || !CGF.getLangOpts().Exceptions ||
-  !CGF.getLangOpts().CXXExceptions ||
-  CGF.Builder.GetInsertBlock() == CGF.AllocaInsertPt->getParent()) {
-if (auto *OMPRegionInfo =
-dyn_cast_or_null(CGF.CapturedStmtInfo)) {
-  if (OMPRegionInfo->getThreadIDVariable()) {
-// Check if this an outlined function with thread id passed as 
argument.
-LValue LVal = OMPRegionInfo->getThreadIDVariableLValue(CGF);
+  if (auto *OMPRegionInfo =
+  dyn_cast_or_null(CGF.CapturedStmtInfo)) {
+if (OMPRegionInfo->getThreadIDVariable()) {
+  // Check if this an outlined function with thread id passed as argument.
+  LValue LVal = OMPRegionInfo->getThreadIDVariableLValue(CGF);
+  llvm::BasicBlock *TopBlock = CGF.AllocaInsertPt->getParent();
+  if (!CGF.EHStack.requiresLandingPad() || !CGF.getLangOpts().Exceptions ||
+  !CGF.getLangOpts().CXXExceptions ||
+  CGF.Builder.GetInsertBlock() == TopBlock ||
+  !isa(LVal.getPointer()) ||
+  cast(LVal.getPointer())->getParent() == TopBlock 
||
+  cast(LVal.getPointer())->getParent() ==
+  CGF.Builder.GetInsertBlock()) {
 ThreadID = CGF.EmitLoadOfScalar(LVal, Loc);
 // If value loaded in entry block, cache it and use it everywhere in
 // function.
-if (CGF.Builder.GetInsertBlock() == CGF.AllocaInsertPt->getParent()) {
+if (CGF.Builder.GetInsertBlock() == TopBlock) {
   auto  = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
   Elem.second.ThreadID = ThreadID;
 }

Modified: cfe/trunk/test/OpenMP/openmp_win_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/openmp_win_codegen.cpp?rev=375134=375133=375134=diff
==
--- cfe/trunk/test/OpenMP/openmp_win_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/openmp_win_codegen.cpp Thu Oct 17 10:12:03 2019
@@ -50,11 +50,10 @@ int main() {
 }
 
 // CHECK: define internal void [[OUTLINED]](
-// CHECK: [[GID:%.+]] = {{.*}}call i32 
@__kmpc_global_thread_num(%struct.ident_t* {{.*}}@0)
 // CHECK: invoke void @{{.+}}foo
 // CHECK: [[CATCHSWITCH:%.+]] = catchswitch within none
 // CHECK: [[CATCHPAD:%.+]] = catchpad within [[CATCHSWITCH]]
-// CHECK: call void @__kmpc_critical(%struct.ident_t* {{.*}}@0, i32 [[GID]],
+// CHECK: call void @__kmpc_critical(%struct.ident_t* {{.*}}@0, i32 
[[GID:%.+]],
 // CHECK: invoke void @{{.+}}bar
 // CHECK: call void @__kmpc_end_critical(%struct.ident_t* {{.*}}@0, i32 
[[GID]],
 // CHECK: catchret from [[CATCHPAD]] to

Modified: cfe/trunk/test/OpenMP/parallel_for_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_for_codegen.cpp?rev=375134=375133=375134=diff
==
--- cfe/trunk/test/OpenMP/parallel_for_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_for_codegen.cpp Thu Oct 17 10:12:03 2019
@@ -40,7 +40,7 @@ void with_var_schedule() {
 // CHECK: [[CHUNK_VAL:%.+]] = load i8, i8* %
 // CHECK: [[CHUNK_SIZE:%.+]] = sext i8 [[CHUNK_VAL]] to i64
 // CHECK: call void @__kmpc_for_static_init_8u([[IDENT_T_TY]]* [[LOOP_LOC]], 
i32 [[GTID:%[^,]+]], i32 33, i32* [[IS_LAST:%[^,]+]], i64* [[OMP_LB:%[^,]+]], 
i64* [[OMP_UB:%[^,]+]], i64* [[OMP_ST:%[^,]+]], i64 1, i64 [[CHUNK_SIZE]])
-// CHECK: call void @__kmpc_for_static_fini([[IDENT_T_TY]]* [[LOOP_LOC]], i32 
[[GTID]])
+// CHECK: call void @__kmpc_for_static_fini([[IDENT_T_TY]]* [[LOOP_LOC]], i32 
[[GTID:%.+]])
 #pragma omp parallel for schedule(static, char(a)) private(a)
   for (unsigned long long i = 1; i < 2 + a; ++i) {
   }
@@ -284,7 +284,7 @@ void test_auto(float *a, float *b, float
 // CHECK: store i32* [[GTID_PARAM_ADDR]], i32** [[GTID_REF_ADDR:%.+]],
 // CHECK: call void @__kmpc_dispatch_init_8([[IDENT_T_TY]]* [[DEFAULT_LOC]], 

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D68554#1712863 , @hans wrote:

> > I can't wait for @hans next Windows Snapshot, this is my gate for rolling 
> > into our builds and CI system, after that, I'm gonna catch every single 
> > person who tries to build/check in unclang-formatted.
>
> As it happens, there's a new snapshot out today :-)


@MyDeveloperDay  awarded this comment a like token!!! (if I could)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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


[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 225455.
MyDeveloperDay added a comment.

Update out by one errors and unused variables


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

https://reviews.llvm.org/D68969

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,7 +18,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -325,12 +324,9 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
  Files, InMemoryFileSystem.get());
 
-  const unsigned ID = Diags->getCustomDiagID(
-  WarningsAsErrors ? clang::DiagnosticsEngine::Error
-   : clang::DiagnosticsEngine::Warning,
-  "code should be clang-formatted [-Wclang-format-violations]");
+  FileManager  = Sources.getFileManager();
+  llvm::ErrorOr FileEntryPtr =
+  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
-  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
   if (WarnFormat && !NoWarnFormat) {
 for (const auto  : Replaces) {
-  Diags->Report(
-  Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()),
-  ID);
+  PresumedLoc PLoc = Sources.getPresumedLoc(
+  
Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()));
+
+  SourceLocation LineBegin =
+  Sources.translateFileLineCol(FileEntryPtr.get(), PLoc.getLine(), 1);
+  SourceLocation NextLineBegin = Sources.translateFileLineCol(
+  FileEntryPtr.get(), PLoc.getLine() + 1, 1);
+
+  const char *StartBuf = Sources.getCharacterData(LineBegin);
+  const char *EndBuf = Sources.getCharacterData(NextLineBegin);
+
+  StringRef Line(StartBuf, (EndBuf - StartBuf) - 1);
+
+  SMDiagnostic Diags(
+  llvm::SourceMgr(), SMLoc(), AssumedFileName, PLoc.getLine(),
+  PLoc.getColumn(),
+  WarningsAsErrors ? SourceMgr::DiagKind::DK_Error
+   : SourceMgr::DiagKind::DK_Warning,
+  "code should be clang-formatted [-Wclang-format-violations]", Line,
+  ArrayRef>());
+
+  Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
   Errors++;
   if (ErrorLimit && Errors >= ErrorLimit)
 break;
 }
   }
-  DiagsBuffer->EndSourceFile();
   return WarningsAsErrors;
 }
 
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,7 +18,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -325,12 +324,9 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
  Files, InMemoryFileSystem.get());
 
-  const unsigned ID = Diags->getCustomDiagID(
-  WarningsAsErrors ? clang::DiagnosticsEngine::Error
-   : 

[PATCH] D68818: [hip][cuda] Fix the extended lambda name mangling issue.

2019-10-17 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 225452.
hliao added a comment.

Force numbering on all lambdas in CUDA/HIP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68818

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST
+// RUN: %clang_cc1 -std=c++11 -x hip -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm %s -o - | FileCheck %s --check-prefix=DEVICE
+
+#include "Inputs/cuda.h"
+
+// HOST: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1
+
+__device__ float d0(float x) {
+  return [](float x) { return x + 2.f; }(x);
+}
+
+__device__ float d1(float x) {
+  return [](float x) { return x * 2.f; }(x);
+}
+
+// DEVICE: amdgpu_kernel void @_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_(
+template 
+__global__ void k0(float *p, F f) {
+  p[0] = f(p[0]) + d0(p[1]) + d1(p[2]);
+}
+
+void f0(float *p) {
+  [](float *p) {
+*p = 1.f;
+  }(p);
+}
+
+// The inner/outer lambdas are required to be mangled following ODR but their
+// linkages are still required to keep the original `internal` linkage.
+
+// HOST: define internal void @_ZZ2f1PfENKUlS_E_clES_(
+// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf(
+void f1(float *p) {
+  [](float *p) {
+k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; });
+  }(p);
+}
+// HOST: @__hip_register_globals
+// HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6223,6 +6223,7 @@
 Record->push_back(Lambda.CaptureDefault);
 Record->push_back(Lambda.NumCaptures);
 Record->push_back(Lambda.NumExplicitCaptures);
+Record->push_back(Lambda.HasKnownInternalLinkage);
 Record->push_back(Lambda.ManglingNumber);
 AddDeclRef(D->getLambdaContextDecl());
 AddTypeSourceInfo(Lambda.MethodTyInfo);
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1690,6 +1690,7 @@
 Lambda.CaptureDefault = Record.readInt();
 Lambda.NumCaptures = Record.readInt();
 Lambda.NumExplicitCaptures = Record.readInt();
+Lambda.HasKnownInternalLinkage = Record.readInt();
 Lambda.ManglingNumber = Record.readInt();
 Lambda.ContextDecl = ReadDeclID();
 Lambda.Captures = (Capture *)Reader.getContext().Allocate(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11431,17 +11431,18 @@
 E->getCaptureDefault());
   getDerived().transformedLocalDecl(OldClass, {Class});
 
-  Optional> Mangling;
+  Optional> Mangling;
   if (getDerived().ReplacingOriginal())
-Mangling = std::make_pair(OldClass->getLambdaManglingNumber(),
-  OldClass->getLambdaContextDecl());
+Mangling = std::make_tuple(OldClass->getLambdaManglingNumber(),
+   OldClass->hasKnownInternalLinkage(),
+   OldClass->getLambdaContextDecl());
 
   // Build the call operator.
   CXXMethodDecl *NewCallOperator = getSema().startLambdaDefinition(
   Class, E->getIntroducerRange(), NewCallOpTSI,
   E->getCallOperator()->getEndLoc(),
   NewCallOpTSI->getTypeLoc().castAs().getParams(),
-  E->getCallOperator()->getConstexprKind(), Mangling);
+  E->getCallOperator()->getConstexprKind());
 
   LSI->CallOperator = NewCallOperator;
 
@@ -11461,6 +11462,9 @@
   getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
   getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
 
+  // Number the lambda for linkage purposes if necessary.
+  getSema().handleLambdaNumbering(Class, NewCallOperator, Mangling);
+
   // Introduce the context of the call operator.
   Sema::ContextRAII SavedContext(getSema(), NewCallOperator,
  /*NewThisContext*/false);
Index: clang/lib/Sema/SemaLambda.cpp
===
--- 

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

2019-10-17 Thread Fabian Maurer via Phabricator via cfe-commits
DarkShadow44 added a comment.

@akhuang Thanks for looking into this!

I've found something else, I've written a small dumper to demonstrate. Pardon 
the long comment please.

Program:

  typedef int* __ptr32 PINT32;
  typedef int* PINT64;
  
  struct s1
  {
PINT32 i32;
PINT64 i64;
  };

AST:

  Kind: 'TranslationUnit(300)', Type: '', Name: 
'/home/fabian/Programming/Wine/wine-git/tools/test.c'
  Kind: 'TypedefDecl(20)', Type: '__ptr32_sptr int *', Name: 'PINT32'
  Kind: 'TypedefDecl(20)', Type: 'PINT64', Name: 'PINT64'
  Kind: 'StructDecl(2)', Type: 'struct s1', Name: 's1'
  Kind: 'FieldDecl(6)', Type: '__ptr32_sptr int *', Name: 'i32'
  Kind: 'TypeRef(43)', Type: '__ptr32_sptr int *', 
Name: 'PINT32'
  Kind: 'FieldDecl(6)', Type: 'PINT64', Name: 'i64'
  Kind: 'TypeRef(43)', Type: 'PINT64', Name: 'PINT64'

Note the difference between

  Type: '__ptr32_sptr int *', Name: 'PINT32'
  Type: 'PINT64', Name: 'PINT64'

3 Issues I have here:

- __ptr32_sptr doesn't exist and seems wrong
- The __ptr32 part should be right of the asterisk
- Why does PINT64 have PINT64 as type, but PINT32 not PINT32?

I'm not sure if this is/should still be part of this patchset or even is an 
issue, just pointing out inconsistencies I noticed - hope I'm not a bother.

Dumper attached, for convenience.
F10298035: dumper.cpp 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66827



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


[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

2019-10-17 Thread David Candler via Phabricator via cfe-commits
dcandler added a comment.

I added the negative option more as a way to disable the flag, since I'm 
currently looking at cases where it may want to be turned on by default (and a 
negative option would then allow you to only get .eh_frame in cases where you'd 
get both .debug_frame/.eh_frame).

The other suggested name of -gdwarf-frame-always might then better describe the 
behavior, as the negative -gno-dwarf-frame-always wouldn't sound quite so 
misleading (since 'not always' equates to 'sometimes').


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

https://reviews.llvm.org/D67216



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


[PATCH] D67550: [AArch64][SVE] Implement unpack intrinsics

2019-10-17 Thread David Greene via Phabricator via cfe-commits
greened accepted this revision.
greened added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D67550



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


Re: r374135 - [c++20] P1152R4: warn on any simple-assignment to a volatile lvalue

2019-10-17 Thread Richard Smith via cfe-commits
On Thu, 17 Oct 2019, 05:17 Stephan Bergmann via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:

> On 09/10/2019 04:04, Richard Smith via cfe-commits wrote:
> > Author: rsmith
> > Date: Tue Oct  8 19:04:54 2019
> > New Revision: 374135
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=374135=rev
> > Log:
> > [c++20] P1152R4: warn on any simple-assignment to a volatile lvalue
> > whose value is not ignored.
> >
> > We don't warn on all the cases that are deprecated: specifically, we
> > choose to not warn for now if there are parentheses around the
> > assignment but its value is not actually used. This seems like a more
> > defensible rule, particularly for cases like sizeof(v = a), where the
> > parens are part of the operand rather than the sizeof syntax.
> >
> > Modified:
> >  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >  cfe/trunk/include/clang/Sema/Sema.h
> >  cfe/trunk/lib/Sema/SemaExpr.cpp
> >  cfe/trunk/lib/Sema/SemaExprCXX.cpp
> >  cfe/trunk/test/SemaCXX/deprecated.cpp
> >  cfe/trunk/www/cxx_status.html
>
> Oh, I should probably have commented here on the mailing list rather
> than at
> <
> https://github.com/llvm/llvm-project/commit/4a6861a7e5b59be24a09b8b9782255d028e7aade#commitcomment-35540755
> >:
>
> I assume the scenario at
>  "Avoid C++20
> deprecated assignment to volatile",
>
>(void) (0 ? *(location) = (result) : 0);
>
> where *(location) is of (non-class) volatile type, is a true positive
> that we indeed want to warn about?
>

Yes, that case is deprecated in C++20.

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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D68377#1698399 , @xbolva00 wrote:

> Current solution does not work
>  /home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: 
> error: redefinition of ‘BImemccpy’
>
>   #define BUILTIN(ID, TYPE, ATTRS) BI##ID,
>^
>
> /home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: 
> note: in definition of macro ‘BUILTIN’
>
>   #define BUILTIN(ID, TYPE, ATTRS) BI##ID,
>   
>
> @aaron.ballman, maybe you can help me how to represent that memccpy exists 
> for GNU and MS?


We may not have the machinery needed for doing this yet, in which case, I think 
it's fine to ignore the MS builtin.


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

https://reviews.llvm.org/D68377



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


[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

2019-10-17 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGccc4d83cda16: [ObjC] Diagnose implicit type coercion from 
ObjC Class to object pointer… (authored by jyknight).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67983

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/comptypes-1.m
  clang/test/SemaObjCXX/class-method-self.mm
  clang/test/SemaObjCXX/comptypes-1.mm
  clang/test/SemaObjCXX/comptypes-7.mm
  clang/test/SemaObjCXX/instancetype.mm

Index: clang/test/SemaObjCXX/instancetype.mm
===
--- clang/test/SemaObjCXX/instancetype.mm
+++ clang/test/SemaObjCXX/instancetype.mm
@@ -5,7 +5,7 @@
 #endif
 
 @interface Root
-+ (instancetype)alloc; // FIXME -- should note {{explicitly declared 'instancetype'}}
++ (instancetype)alloc; // expected-note {{explicitly declared 'instancetype'}}
 - (instancetype)init; // expected-note{{overridden method is part of the 'init' method family}}
 - (instancetype)self; // expected-note {{explicitly declared 'instancetype'}}
 - (Class)class;
@@ -143,7 +143,7 @@
 
 @implementation Subclass4
 + (id)alloc {
-  return self; // FIXME -- should error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}}
+  return self; // expected-error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}}
 }
 
 - (Subclass3 *)init { return 0; } // don't complain: we lost the related return type
Index: clang/test/SemaObjCXX/comptypes-7.mm
===
--- clang/test/SemaObjCXX/comptypes-7.mm
+++ clang/test/SemaObjCXX/comptypes-7.mm
@@ -47,23 +47,23 @@
 
   if (obj == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}}
   if (i == obj) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}}
-  if (obj == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}}
-  if (j == obj) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}}
+  if (obj == j) foo() ; // expected-error {{comparison of distinct pointer types ('id' and 'int *')}}
+  if (j == obj) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id')}}
 
   if (obj_c == i) foo() ; // expected-error {{comparison between pointer and integer ('MyClass *' and 'int')}}
   if (i == obj_c) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'MyClass *')}}
-  if (obj_c == j) foo() ; // expected-error {{invalid operands to binary expression ('MyClass *' and 'int *')}}
-  if (j == obj_c) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'MyClass *')}}
+  if (obj_c == j) foo() ; // expected-error {{comparison of distinct pointer types ('MyClass *' and 'int *')}}
+  if (j == obj_c) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'MyClass *')}}
 
   if (obj_p == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}}
   if (i == obj_p) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}}
-  if (obj_p == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}}
-  if (j == obj_p) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}}
+  if (obj_p == j) foo() ; // expected-error {{comparison of distinct pointer types ('id' and 'int *')}}
+  if (j == obj_p) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id')}}
 
   if (obj_C == i) foo() ; // expected-error {{comparison between pointer and integer ('Class' and 'int')}}
   if (i == obj_C) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'Class')}}
-  if (obj_C == j) foo() ; // expected-error {{invalid operands to binary expression ('Class' and 'int *')}}
-  if (j == obj_C) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'Class')}}
+  if (obj_C == j) foo() ; // expected-error {{comparison of distinct pointer types ('Class' and 'int *')}}
+  if (j == obj_C) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'Class')}}
 
   Class bar1 = nil;
   Class  bar = nil;
Index: clang/test/SemaObjCXX/comptypes-1.mm
===
--- clang/test/SemaObjCXX/comptypes-1.mm
+++ clang/test/SemaObjCXX/comptypes-1.mm
@@ -38,7 +38,7 @@
   obj_c = obj;/* Ok */
   obj_c = obj_p;  // expected-error {{assigning to 'MyClass *' from incompatible type 'id'}}
   obj_c = obj_cp; // expected-error {{assigning to 'MyClass *' from incompatible type 'MyOtherClass *'}}
-  obj_c = obj_C;  // FIXME -- should error {{assigning to 'MyClass *' from incompatible type 

r375125 - [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object

2019-10-17 Thread James Y Knight via cfe-commits
Author: jyknight
Date: Thu Oct 17 08:27:04 2019
New Revision: 375125

URL: http://llvm.org/viewvc/llvm-project?rev=375125=rev
Log:
[ObjC] Diagnose implicit type coercion from ObjC 'Class' to object
pointer types.

For example, in Objective-C mode, the initialization of 'x' in:
```
  @implementation MyType
  + (void)someClassMethod {
MyType *x = self;
  }
  @end
```
is correctly diagnosed with an incompatible-pointer-types warning, but
in Objective-C++ mode, it is not diagnosed at all -- even though
incompatible pointer conversions generally become an error in C++.

This patch fixes that oversight, allowing implicit conversions
involving Class only to/from unqualified-id, and between qualified and
unqualified Class, where the protocols are compatible.

Note that this does change some behaviors in Objective-C, as well, as
shown by the modified tests.

Of particular note is that assignment from from 'Class' to
'id' now warns. (Despite appearances, those are not
compatible types. 'Class' is not expected to have instance
methods defined by 'MyProtocol', while 'id' is.)

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

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaObjC/comptypes-1.m
cfe/trunk/test/SemaObjCXX/class-method-self.mm
cfe/trunk/test/SemaObjCXX/comptypes-1.mm
cfe/trunk/test/SemaObjCXX/comptypes-7.mm
cfe/trunk/test/SemaObjCXX/instancetype.mm

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=375125=375124=375125=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Oct 17 08:27:04 2019
@@ -8025,14 +8025,15 @@ bool ASTContext::ObjCQualifiedClassTypes
 bool ASTContext::ObjCQualifiedIdTypesAreCompatible(
 const ObjCObjectPointerType *lhs, const ObjCObjectPointerType *rhs,
 bool compare) {
-  // Allow id and an 'id' or void* type in all cases.
-  if (lhs->isVoidPointerType() ||
-  lhs->isObjCIdType() || lhs->isObjCClassType())
-return true;
-  else if (rhs->isVoidPointerType() ||
-   rhs->isObjCIdType() || rhs->isObjCClassType())
+  // Allow id and an 'id' in all cases.
+  if (lhs->isObjCIdType() || rhs->isObjCIdType())
 return true;
 
+  // Don't allow id to convert to Class or Class in either direction.
+  if (lhs->isObjCClassType() || lhs->isObjCQualifiedClassType() ||
+  rhs->isObjCClassType() || rhs->isObjCQualifiedClassType())
+return false;
+
   if (lhs->isObjCQualifiedIdType()) {
 if (rhs->qual_empty()) {
   // If the RHS is a unqualified interface pointer "NSString*",
@@ -8142,9 +8143,8 @@ bool ASTContext::canAssignObjCInterfaces
   const ObjCObjectType* LHS = LHSOPT->getObjectType();
   const ObjCObjectType* RHS = RHSOPT->getObjectType();
 
-  // If either type represents the built-in 'id' or 'Class' types, return true.
-  if (LHS->isObjCUnqualifiedIdOrClass() ||
-  RHS->isObjCUnqualifiedIdOrClass())
+  // If either type represents the built-in 'id' type, return true.
+  if (LHS->isObjCUnqualifiedId() || RHS->isObjCUnqualifiedId())
 return true;
 
   // Function object that propagates a successful result or handles
@@ -8162,14 +8162,22 @@ bool ASTContext::canAssignObjCInterfaces
LHSOPT->stripObjCKindOfTypeAndQuals(*this));
   };
 
+  // Casts from or to id are allowed when the other side has compatible
+  // protocols.
   if (LHS->isObjCQualifiedId() || RHS->isObjCQualifiedId()) {
 return finish(ObjCQualifiedIdTypesAreCompatible(LHSOPT, RHSOPT, false));
   }
 
+  // Verify protocol compatibility for casts from Class to Class.
   if (LHS->isObjCQualifiedClass() && RHS->isObjCQualifiedClass()) {
 return finish(ObjCQualifiedClassTypesAreCompatible(LHSOPT, RHSOPT));
   }
 
+  // Casts from Class to Class, or vice-versa, are allowed.
+  if (LHS->isObjCClass() && RHS->isObjCClass()) {
+return true;
+  }
+
   // If we have 2 user-defined types, fall into that path.
   if (LHS->getInterface() && RHS->getInterface()) {
 return finish(canAssignObjCInterfaces(LHS, RHS));

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=375125=375124=375125=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Oct 17 08:27:04 2019
@@ -10068,8 +10068,8 @@ static bool convertPointersToCompositeTy
 
   QualType T = S.FindCompositePointerType(Loc, LHS, RHS);
   if (T.isNull()) {
-if ((LHSType->isPointerType() || LHSType->isMemberPointerType()) &&
-(RHSType->isPointerType() || RHSType->isMemberPointerType()))
+if ((LHSType->isAnyPointerType() || LHSType->isMemberPointerType()) &&
+(RHSType->isAnyPointerType() || RHSType->isMemberPointerType()))
  

[PATCH] D68028: [clang] Add no_builtin attribute

2019-10-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3416
+
+def NoBuiltin : InheritableAttr {
+  let Spellings = [Clang<"no_builtin">];

I think we do not want this to be an inheritable attribute, but just `Attr` 
instead, because this can only be written on definitions (so there's no way to 
inherit the attribute from previous declarations).



Comment at: clang/test/Sema/no-builtin.c:26
+int __attribute__((no_builtin("*"))) variable;
+// expected-warning@-1 {{'no_builtin' attribute only applies to functions}}

gchatelet wrote:
> aaron.ballman wrote:
> > You should probably add another test case for this situation:
> > ```
> > void whatever() __attribute__((no_builtin("*", "*"))) {}
> > ```
> > and for member functions in C++ as well:
> > ```
> > struct S {
> >   void whatever() __attribute__((no_builtin("memcpy"))) {}
> >   virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
> > };
> > ```
> ```
> void whatever() __attribute__((no_builtin("*", "*"))) {}
> ```
> 
> I've added a few ones.
> 
> ```
> struct S {
>   void whatever() __attribute__((no_builtin("memcpy"))) {}
>   virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
> };
> ```
> 
> What do you want me to test for this one?
> What do you want me to test for this one?

Ensuring that applying the attribute to member function definitions, including 
virtual ones, is supported and doesn't cause diagnostics. The codegen side of 
things will test that overridden virtual methods do not inherit the attribute.

Actually, there's another interesting test hiding in there for when we do want 
a diagnostic (I think):
```
struct S {
  void whatever() __attribute__((no_builtin("memcpy"))); // Should diagnose as 
not a definition
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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


[PATCH] D67982: [ObjC] Add some additional test cases around pointer conversions.

2019-10-17 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c982af05997: [ObjC] Add some additional test cases around 
pointer conversions. (authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D67982?vs=221586=225439#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67982

Files:
  clang/test/SemaObjC/class-method-self.m
  clang/test/SemaObjC/comptypes-1.m
  clang/test/SemaObjC/comptypes-7.m
  clang/test/SemaObjCXX/class-method-self.mm
  clang/test/SemaObjCXX/comptypes-1.mm
  clang/test/SemaObjCXX/comptypes-7.mm
  clang/test/SemaObjCXX/instancetype.mm

Index: clang/test/SemaObjCXX/instancetype.mm
===
--- clang/test/SemaObjCXX/instancetype.mm
+++ clang/test/SemaObjCXX/instancetype.mm
@@ -5,7 +5,7 @@
 #endif
 
 @interface Root
-+ (instancetype)alloc;
++ (instancetype)alloc; // FIXME -- should note {{explicitly declared 'instancetype'}}
 - (instancetype)init; // expected-note{{overridden method is part of the 'init' method family}}
 - (instancetype)self; // expected-note {{explicitly declared 'instancetype'}}
 - (Class)class;
@@ -143,7 +143,7 @@
 
 @implementation Subclass4
 + (id)alloc {
-  return self; // FIXME: we accept this in ObjC++ but not ObjC?
+  return self; // FIXME -- should error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}}
 }
 
 - (Subclass3 *)init { return 0; } // don't complain: we lost the related return type
Index: clang/test/SemaObjCXX/comptypes-7.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/comptypes-7.mm
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
+
+#define nil nullptr
+
+extern void foo();
+
+@protocol MyProtocol
+- (void) method;
+@end
+
+@interface MyClass
+@end
+
+int main()
+{
+  id obj = nil;
+  id  obj_p = nil;
+  MyClass *obj_c = nil;
+  Class obj_C = nil;
+
+  int i = 0;
+  int *j = nil;
+
+  /* These should all generate errors.  */
+
+  obj = i; // expected-error {{assigning to 'id' from incompatible type 'int'}}
+  obj = j; // expected-error {{assigning to 'id' from incompatible type 'int *'}}
+
+  obj_p = i; // expected-error {{assigning to 'id' from incompatible type 'int'}}
+  obj_p = j; // expected-error {{assigning to 'id' from incompatible type 'int *'}}
+
+  obj_c = i; // expected-error {{assigning to 'MyClass *' from incompatible type 'int'}}
+  obj_c = j; // expected-error {{assigning to 'MyClass *' from incompatible type 'int *'}}
+
+  obj_C = i; // expected-error {{assigning to 'Class' from incompatible type 'int'}}
+  obj_C = j; // expected-error {{assigning to 'Class' from incompatible type 'int *'}}
+
+  i = obj;   // expected-error {{assigning to 'int' from incompatible type 'id'}}
+  i = obj_p; // expected-error {{assigning to 'int' from incompatible type 'id'}}
+  i = obj_c; // expected-error {{assigning to 'int' from incompatible type 'MyClass *'}}
+  i = obj_C; // expected-error {{assigning to 'int' from incompatible type 'Class'}}
+
+  j = obj;   // expected-error {{assigning to 'int *' from incompatible type 'id'}}
+  j = obj_p; // expected-error {{assigning to 'int *' from incompatible type 'id'}}
+  j = obj_c; // expected-error {{assigning to 'int *' from incompatible type 'MyClass *'}}
+  j = obj_C; // expected-error {{assigning to 'int *' from incompatible type 'Class'}}
+
+  if (obj == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}}
+  if (i == obj) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}}
+  if (obj == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}}
+  if (j == obj) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}}
+
+  if (obj_c == i) foo() ; // expected-error {{comparison between pointer and integer ('MyClass *' and 'int')}}
+  if (i == obj_c) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'MyClass *')}}
+  if (obj_c == j) foo() ; // expected-error {{invalid operands to binary expression ('MyClass *' and 'int *')}}
+  if (j == obj_c) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'MyClass *')}}
+
+  if (obj_p == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}}
+  if (i == obj_p) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}}
+  if (obj_p == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}}
+  if (j == obj_p) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}}
+
+  if (obj_C == i) foo() ; // expected-error {{comparison between pointer and integer ('Class' and 'int')}}
+  if (i == obj_C) foo() ; // expected-error {{comparison between 

r375124 - [ObjC] Add some additional test cases around pointer conversions.

2019-10-17 Thread James Y Knight via cfe-commits
Author: jyknight
Date: Thu Oct 17 08:18:59 2019
New Revision: 375124

URL: http://llvm.org/viewvc/llvm-project?rev=375124=rev
Log:
[ObjC] Add some additional test cases around pointer conversions.

This is especially important for Objective-C++, which is entirely
missing this testing at the moment.

This annotates with "FIXME" the cases which I change in the next
patch -- I primarily wanted to document the current state of things so
that the effect of the code change is made clear.

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

Added:
cfe/trunk/test/SemaObjCXX/class-method-self.mm
cfe/trunk/test/SemaObjCXX/comptypes-1.mm
cfe/trunk/test/SemaObjCXX/comptypes-7.mm
Modified:
cfe/trunk/test/SemaObjC/class-method-self.m
cfe/trunk/test/SemaObjC/comptypes-1.m
cfe/trunk/test/SemaObjC/comptypes-7.m
cfe/trunk/test/SemaObjCXX/instancetype.mm

Modified: cfe/trunk/test/SemaObjC/class-method-self.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/class-method-self.m?rev=375124=375123=375124=diff
==
--- cfe/trunk/test/SemaObjC/class-method-self.m (original)
+++ cfe/trunk/test/SemaObjC/class-method-self.m Thu Oct 17 08:18:59 2019
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-objc-root-class %s
 
-typedef struct objc_class *Class;
 @interface XX
 
 - (void)addObserver:(XX*)o; // expected-note 2{{passing argument to parameter 
'o' here}}
@@ -23,4 +22,3 @@ static XX *obj;
   [obj addObserver:whatever]; // expected-warning {{incompatible pointer types 
sending 'Class' to parameter of type 'XX *'}}
 }
 @end
-

Modified: cfe/trunk/test/SemaObjC/comptypes-1.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/comptypes-1.m?rev=375124=375123=375124=diff
==
--- cfe/trunk/test/SemaObjC/comptypes-1.m (original)
+++ cfe/trunk/test/SemaObjC/comptypes-1.m Thu Oct 17 08:18:59 2019
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
 
 #define nil (void *)0;
-#define Nil (void *)0;
 
 extern void foo();
 
 @protocol MyProtocol
 - (void) foo;
++ (void) bar;
 @end
 
 @interface MyClass
@@ -22,7 +22,8 @@ int main()
   id obj_p = nil;
   MyClass *obj_c = nil;
   MyOtherClass *obj_cp = nil;
-  Class obj_C = Nil;
+  Class obj_C = nil;
+  Class obj_CP = nil;
 
   /* Assigning to an 'id' variable should never
  generate a warning.  */
@@ -30,12 +31,15 @@ int main()
   obj = obj_c;  /* Ok  */
   obj = obj_cp; /* Ok  */
   obj = obj_C;  /* Ok  */
-  
+  obj = obj_CP;  /* Ok  */
+
   /* Assigning to a 'MyClass *' variable should always generate a
  warning, unless done from an 'id'.  */
   obj_c = obj;/* Ok */
-  obj_c = obj_cp; // // expected-warning {{incompatible pointer types 
assigning to 'MyClass *' from 'MyOtherClass *'}}
+  obj_c = obj_p;  // expected-warning {{assigning to 'MyClass *' from 
incompatible type 'id'}}
+  obj_c = obj_cp; // expected-warning {{incompatible pointer types assigning 
to 'MyClass *' from 'MyOtherClass *'}}
   obj_c = obj_C;  // expected-warning {{incompatible pointer types assigning 
to 'MyClass *' from 'Class'}}
+  obj_c = obj_CP; // expected-warning {{incompatible pointer types assigning 
to 'MyClass *' from 'Class'}}
 
   /* Assigning to an 'id' variable should generate a
  warning if done from a 'MyClass *' (which doesn't implement
@@ -45,6 +49,7 @@ int main()
   obj_p = obj_c;  // expected-warning {{assigning to 'id' from 
incompatible type 'MyClass *'}}
   obj_p = obj_cp; /* Ok  */
   obj_p = obj_C;  // expected-warning {{incompatible pointer types assigning 
to 'id' from 'Class'}}
+  obj_p = obj_CP; // FIXME -- should warn {{assigning to 'id' from 
incompatible type 'Class'}}
 
   /* Assigning to a 'MyOtherClass *' variable should always generate
  a warning, unless done from an 'id' or an 'id' (since
@@ -53,37 +58,67 @@ int main()
   obj_cp = obj_c;  // expected-warning {{incompatible pointer types assigning 
to 'MyOtherClass *' from 'MyClass *'}}
   obj_cp = obj_p;  /* Ok */
   obj_cp = obj_C;  // expected-warning {{incompatible pointer types assigning 
to 'MyOtherClass *' from 'Class'}}
+  obj_cp = obj_CP; // expected-warning {{incompatible pointer types assigning 
to 'MyOtherClass *' from 'Class'}}
+
+  obj_C = obj; // Ok
+  obj_C = obj_p;   // expected-warning {{incompatible pointer types assigning 
to 'Class' from 'id'}}
+  obj_C = obj_c;   // expected-warning {{incompatible pointer types assigning 
to 'Class' from 'MyClass *'}}
+  obj_C = obj_cp;  // expected-warning {{incompatible pointer types assigning 
to 'Class' from 'MyOtherClass *'}}
+  obj_C = obj_CP;  // Ok
+
+  obj_CP = obj; // Ok
+  obj_CP = obj_p;   // expected-warning {{assigning to 'Class' 
from incompatible type 'id'}}
+  obj_CP = obj_c;   // expected-warning {{incompatible pointer types assigning 
to 'Class' from 'MyClass *}}
+  obj_CP = obj_cp;  // expected-warning 

[PATCH] D68028: [clang] Add no_builtin attribute

2019-10-17 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 225435.
gchatelet marked 13 inline comments as done.
gchatelet added a comment.
Herald added subscribers: s.egerton, simoncook, aheejin, dschuff.

- Address comments
- Fix missing rename
- Address comments
- Add warning category


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/no-builtin.cpp

Index: clang/test/Sema/no-builtin.cpp
===
--- /dev/null
+++ clang/test/Sema/no-builtin.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+
+/// Prevent use of all builtins.
+void valid_attribute_all_1() __attribute__((no_builtin)) {}
+void valid_attribute_all_2() __attribute__((no_builtin())) {}
+
+/// Prevent use of specific builtins.
+void valid_attribute_function() __attribute__((no_builtin("memcpy"))) {}
+void valid_attribute_functions() __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcmp"))) {}
+
+/// Many times the same builtin is fine.
+void many_attribute_function_1() __attribute__((no_builtin)) __attribute__((no_builtin)) {}
+void many_attribute_function_2() __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcpy"))) {}
+void many_attribute_function_3() __attribute__((no_builtin("memcpy", "memcpy"))) {}
+void many_attribute_function_4() __attribute__((no_builtin("memcpy", "memcpy"))) __attribute__((no_builtin("memcpy"))) {}
+
+/// Invalid builtin name.
+void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {}
+// expected-warning@-1 {{'not_a_builtin' is not a valid builtin name for no_builtin}}
+
+/// Can't use bare no_builtin with a named one.
+void wildcard_and_functionname() __attribute__((no_builtin)) __attribute__((no_builtin("memcpy"))) {}
+// expected-error@-1 {{empty no_builtin cannot be composed with named ones}}
+
+/// Can't attach attribute to a variable.
+int __attribute__((no_builtin)) variable;
+// expected-warning@-1 {{'no_builtin' attribute only applies to functions}}
+
+/// Can't attach attribute to a declaration.
+void nobuiltin_on_declaration() __attribute__((no_builtin));
+// expected-error@-1 {{no_builtin attribute is permitted on definitions only}}
+
+struct S {
+  /// Can't attach attribute to a defaulted function,
+  S()
+  __attribute__((no_builtin)) = default;
+  // expected-error@-1 {{no_builtin attribute has no effect on defaulted or deleted functions}}
+
+  /// Can't attach attribute to a deleted function,
+  S(const S &)
+  __attribute__((no_builtin)) = delete;
+  // expected-error@-1 {{no_builtin attribute has no effect on defaulted or deleted functions}}
+};
+
+/// Can't attach attribute to an aliased function.
+void alised_function() {}
+void aliasing_function() __attribute__((no_builtin)) __attribute__((alias("alised_function")));
+// expected-error@-1 {{no_builtin attribute is permitted on definitions only}}
\ No newline at end of file
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
@@ -74,6 +74,7 @@
 // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method)
 // CHECK-NEXT: Naked (SubjectMatchRule_function)
+// CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
Index: clang/test/CodeGen/no-builtin.cpp
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @foo_no_mempcy() #0
+extern "C" void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_twice() #0
+extern "C" void foo_no_mempcy_twice() __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_builtins() #1
+extern "C" void foo_no_builtins() __attribute__((no_builtin)) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_memset() #2
+extern "C" void foo_no_mempcy_memset() 

[PATCH] D68028: [clang] Add no_builtin attribute

2019-10-17 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3600
   "attribute">;
+def err_attribute_no_builtin_invalid_builtin_name : Error<
+  "'%0' is not a valid builtin name for %1">;

aaron.ballman wrote:
> I think this should be a warning rather than an error.
> 
> Imagine the situation where the user uses Clang 11 to write their code and 
> they disable a Clang 11-specific builtin from being used, but then they try 
> to compile the code in Clang 10 which doesn't know about the builtin.
> 
> Rather than err, it seems reasonable to warn about the unknown builtin name 
> (under a new warning flag group under `-Wattributes`) and then just ignore 
> the attribute. Because the builtin is unknown anyway, we won't transform any 
> constructs to use it.
Makes perfect sense, thx for pointing this out.



Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510
+  // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin
+  // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which
+  // always returns false before Sema::ActOnStartOfFunctionDef is called.

rsmith wrote:
> aaron.ballman wrote:
> > rsmith wrote:
> > > aaron.ballman wrote:
> > > > handleNoBuiltin -> handleNoBuiltinAttr
> > > I am not convinced that this is a bug -- the function declaration does 
> > > not become a definition until the parser reaches the definition.
> > > 
> > > In any case, I don't think the predicate you want is "is this declaration 
> > > a definition?". Rather, I think what you want is, "does this declaration 
> > > introduce an explicit function body?". In particular, we should not 
> > > permit uses of this attribute on defaulted or deleted functions, nor on 
> > > functions that have a definition by virtue of using 
> > > `__attribute__((alias))`. So it probably should be a syntactic check on 
> > > the form of the declarator (that is, the check you're perrforming here), 
> > > and the check should probably be `D.getFunctionDefinitionKind() == 
> > > FDK_Definition`. (A custom diagnostic for using the attribute on a 
> > > defaulted or deleted function would be nice too, since the existing 
> > > diagnostic text isn't really accurate in those cases.)
> > > In particular, we should not permit uses of this attribute on defaulted 
> > > or deleted functions
> > 
> > Deleted functions, sure. Defaulted functions... not so sure. I could sort 
> > of imagine wanting to instruct a defaulted assignment operator that does 
> > memberwise copy that it's not allowed to use a builtin memcpy, for 
> > instance. Or is this a bad idea for some reason I'm not thinking of?
> `-fno-builtin` does not turn off using `llvm.memcpy` for copying memory, and 
> it doesn't turn off `llvm.memcpy` being lowered to a call to `memcpy`. 
> Allowing this for defaulted functions would only give a false sense of 
> security, at least for now (though I could imagine we might find some way to 
> change that in the future).
> 
> Also, trivial defaulted functions (where we're most likely to end up with 
> `memcpy` calls) are often not emitted at all, instead being directly inlined 
> by the frontend, so there's nowhere to attach a `no-builtin-memcpy` LLVM 
> attribute (we'd need to put the attribute on all callers of those functions) 
> even if LLVM learned to not emit calls to `memcpy` to implement `llvm.memcpy` 
> when operating under a `no-builtin-memcpy` constraint.
> I am not convinced that this is a bug -- the function declaration does not 
> become a definition until the parser reaches the definition.

@rsmith It may not be a bug but it is currently used in a buggy manner (See 
D68379).
An assert to prevent misuse would be welcome IMHO, I've added a note on the 
function meanwhile.



Comment at: clang/test/Sema/no-builtin.c:26
+int __attribute__((no_builtin("*"))) variable;
+// expected-warning@-1 {{'no_builtin' attribute only applies to functions}}

aaron.ballman wrote:
> You should probably add another test case for this situation:
> ```
> void whatever() __attribute__((no_builtin("*", "*"))) {}
> ```
> and for member functions in C++ as well:
> ```
> struct S {
>   void whatever() __attribute__((no_builtin("memcpy"))) {}
>   virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
> };
> ```
```
void whatever() __attribute__((no_builtin("*", "*"))) {}
```

I've added a few ones.

```
struct S {
  void whatever() __attribute__((no_builtin("memcpy"))) {}
  virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
};
```

What do you want me to test for this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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


r375119 - [OPENMP]Fix thread id passed to outlined region in sequential parallel

2019-10-17 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Oct 17 07:36:43 2019
New Revision: 375119

URL: http://llvm.org/viewvc/llvm-project?rev=375119=rev
Log:
[OPENMP]Fix thread id passed to outlined region in sequential parallel
regions.

The real global thread id must be passed to the outlined region instead
of the zero thread id.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/parallel_if_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=375119=375118=375119=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Oct 17 07:36:43 2019
@@ -3075,17 +3075,15 @@ void CGOpenMPRuntime::emitParallelCall(C
 CGF.EmitRuntimeCall(
 RT.createRuntimeFunction(OMPRTL__kmpc_serialized_parallel), Args);
 
-// OutlinedFn(, _bound, CapturedStruct);
-Address ZeroAddr = CGF.CreateDefaultAlignTempAlloca(CGF.Int32Ty,
-/*Name=*/".zero.addr");
-CGF.InitTempAlloca(ZeroAddr, CGF.Builder.getInt32(/*C*/ 0));
+// OutlinedFn(, _bound, CapturedStruct);
+Address ThreadIDAddr = RT.emitThreadIDAddress(CGF, Loc);
 Address ZeroAddrBound =
 CGF.CreateDefaultAlignTempAlloca(CGF.Int32Ty,
  /*Name=*/".bound.zero.addr");
 CGF.InitTempAlloca(ZeroAddrBound, CGF.Builder.getInt32(/*C*/ 0));
 llvm::SmallVector OutlinedFnArgs;
 // ThreadId for serialized parallels is 0.
-OutlinedFnArgs.push_back(ZeroAddr.getPointer());
+OutlinedFnArgs.push_back(ThreadIDAddr.getPointer());
 OutlinedFnArgs.push_back(ZeroAddrBound.getPointer());
 OutlinedFnArgs.append(CapturedVars.begin(), CapturedVars.end());
 RT.emitOutlinedFunctionCall(CGF, Loc, OutlinedFn, OutlinedFnArgs);

Modified: cfe/trunk/test/OpenMP/parallel_if_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_if_codegen.cpp?rev=375119=375118=375119=diff
==
--- cfe/trunk/test/OpenMP/parallel_if_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_if_codegen.cpp Thu Oct 17 07:36:43 2019
@@ -30,12 +30,12 @@ void gtid_test() {
 
 // CHECK: define internal {{.*}}void [[GTID_TEST_REGION1]](i{{.+}}* noalias 
[[GTID_PARAM:%.+]], i32* noalias
 // CHECK: store i32 0, i32* [[BND_ZERO_ADDR:%.+]],
-// CHECK: store i32 0, i32* [[ZERO_ADDR:%.+]],
 // CHECK: store i{{[0-9]+}}* [[GTID_PARAM]], i{{[0-9]+}}** 
[[GTID_ADDR_REF:%.+]],
 // CHECK: [[GTID_ADDR:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** 
[[GTID_ADDR_REF]]
 // CHECK: [[GTID:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[GTID_ADDR]]
 // CHECK: call {{.*}}void @__kmpc_serialized_parallel(%{{.+}}* @{{.+}}, 
i{{.+}} [[GTID]])
-// CHECK: call void [[GTID_TEST_REGION2:@.+]](i{{[0-9]+}}* [[ZERO_ADDR]], 
i{{[0-9]+}}* [[BND_ZERO_ADDR]]
+// CHECK: [[GTID_ADDR:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** 
[[GTID_ADDR_REF]]
+// CHECK: call void [[GTID_TEST_REGION2:@.+]](i{{[0-9]+}}* [[GTID_ADDR]], 
i{{[0-9]+}}* [[BND_ZERO_ADDR]]
 // CHECK: call {{.*}}void @__kmpc_end_serialized_parallel(%{{.+}}* @{{.+}}, 
i{{.+}} [[GTID]])
 // CHECK: ret void
 
@@ -57,15 +57,14 @@ int tmain(T Arg) {
 // CHECK-LABEL: define {{.*}}i{{[0-9]+}} @main()
 int main() {
 // CHECK: store i32 0, i32* [[BND_ZERO_ADDR2:%.+]],
-// CHECK: store i32 0, i32* [[ZERO_ADDR2:%.+]],
 // CHECK: store i32 0, i32* [[BND_ZERO_ADDR1:%.+]],
-// CHECK: store i32 0, i32* [[ZERO_ADDR1:%.+]],
 // CHECK: [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num(
 // CHECK: call {{.*}}void {{.+}} @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{.+}} 
0, void {{.+}}* [[CAP_FN4:@.+]] to void
 #pragma omp parallel if (true)
   fn4();
 // CHECK: call {{.*}}void @__kmpc_serialized_parallel(%{{.+}}* @{{.+}}, i32 
[[GTID]])
-// CHECK: call void [[CAP_FN5:@.+]](i32* [[ZERO_ADDR1]], i32* 
[[BND_ZERO_ADDR1]])
+// CHECK: store i32 [[GTID]], i32* [[GTID_ADDR:%.+]],
+// CHECK: call void [[CAP_FN5:@.+]](i32* [[GTID_ADDR]], i32* 
[[BND_ZERO_ADDR1]])
 // CHECK: call {{.*}}void @__kmpc_end_serialized_parallel(%{{.+}}* @{{.+}}, 
i32 [[GTID]])
 #pragma omp parallel if (false)
   fn5();
@@ -76,7 +75,8 @@ int main() {
 // CHECK: br label %[[OMP_END:.+]]
 // CHECK: [[OMP_ELSE]]
 // CHECK: call {{.*}}void @__kmpc_serialized_parallel(%{{.+}}* @{{.+}}, i32 
[[GTID]])
-// CHECK: call void [[CAP_FN6]](i32* [[ZERO_ADDR2]], i32* [[BND_ZERO_ADDR2]])
+// CHECK: store i32 [[GTID]], i32* [[GTID_ADDR:%.+]],
+// CHECK: call void [[CAP_FN6]](i32* [[GTID_ADDR]], i32* [[BND_ZERO_ADDR2]])
 // CHECK: call {{.*}}void @__kmpc_end_serialized_parallel(%{{.+}}* @{{.+}}, 
i32 [[GTID]])
 // CHECK: br label %[[OMP_END]]
 // CHECK: [[OMP_END]]
@@ -100,13 +100,12 @@ int main() {
 
 // CHECK-LABEL: define {{.+}} @{{.+}}tmain
 // CHECK: store i32 0, i32* [[BND_ZERO_ADDR2:%.+]],
-// 

[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2280
 }
-EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls));
+EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls))
+<< File.code();

ilya-biryukov wrote:
> This looks unrelated. Maybe revert and land separately?
This patch also modifies the `getNonLocalDeclRefs` which is related to the test 
here. I'd keep the change in this patch as it is minor and NFC. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977



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


[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 225428.
hokein marked 7 inline comments as done.
hokein added a comment.

address remaining nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2277,7 +2277,8 @@
   if (const auto *ND = llvm::dyn_cast(D))
 Names.push_back(ND->getQualifiedNameAsString());
 }
-EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls));
+EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls))
+<< File.code();
   }
 }
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -600,59 +600,67 @@
   }
 )cpp",
"0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   "1: targets = {ns::global}, qualifier = 'ns::', decl\n"},
   // Simple types.
   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
-   $0^Struct x;
-   $1^Typedef y;
-   static_cast<$2^Struct*>(0);
+   $0^Struct $1^x;
+   $2^Typedef $3^y;
+   static_cast<$4^Struct*>(0);
  }
)cpp",
"0: targets = {Struct}\n"
-   "1: targets = {Typedef}\n"
-   "2: targets = {Struct}\n"},
+   "1: targets = {x}, decl\n"
+   "2: targets = {Typedef}\n"
+   "3: targets = {y}, decl\n"
+   "4: targets = {Struct}\n"},
   // Name qualifiers.
   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  void foo() {
-   $0^a::$1^b::$2^S x;
-   using namespace $3^a::$4^b;
-   $5^S::$6^type y;
+   $0^a::$1^b::$2^S $3^x;
+   using namespace $4^a::$5^b;
+   $6^S::$7^type $8^y;
  }
 )cpp",
"0: targets = {a}\n"
"1: targets = {a::b}, qualifier = 'a::'\n"
"2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-   "3: targets = {a}\n"
-   "4: targets = {a::b}, qualifier = 'a::'\n"
-   "5: targets = {a::b::S}\n"
-   "6: targets = {a::b::S::type}, qualifier = 'struct S::'\n"},
+   "3: targets = {x}, decl\n"
+   "4: targets = {a}\n"
+   "5: targets = {a::b}, qualifier = 'a::'\n"
+   "6: targets = {a::b::S}\n"
+   "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
+   "8: targets = {y}, decl\n"},
   // Simple templates.
   {R"cpp(
   template  struct vector { using value_type = T; };
   template <> struct vector { using value_type = bool; };
   void foo() {
-$0^vector vi;
-$1^vector vb;
+$0^vector $1^vi;
+$2^vector $3^vb;
   }
 )cpp",
"0: targets = {vector}\n"
-   "1: targets = {vector}\n"},
+   "1: targets = {vi}, decl\n"
+   "2: targets = {vector}\n"
+   "3: targets = {vb}, decl\n"},
   // Template type aliases.
   {R"cpp(
 template  struct vector { using value_type = T; };
 template <> struct vector { using value_type = bool; };
 template  using valias = vector;
 void foo() {
-  $0^valias vi;
-  $1^valias vb;
+  $0^valias $1^vi;
+  $2^valias $3^vb;
 }
   )cpp",
"0: targets = {valias}\n"
-   "1: targets = {valias}\n"},
+   "1: targets = {vi}, decl\n"
+   "2: targets = {valias}\n"
+   "3: targets = {vb}, decl\n"},
   // MemberExpr should know their using declaration.
   {R"cpp(
 struct X { void func(int); }
@@ -694,13 +702,14 @@
 };
 
 void foo() {
-  for (int x : $0^vector()) {
-$1^x = 10;
+  for (int $0^x : $1^vector()) {
+$2^x = 10;
   }
 }
 )cpp",
-   "0: targets = {vector}\n"
-   "1: targets = {x}\n"},
+   "0: targets = {x}, decl\n"
+   "1: targets = {vector}\n"
+   "2: targets = {x}\n"},
   // Handle UnresolvedLookupExpr.
 

[PATCH] D68403: [OpenCL] PR43145: preserve addrspace for class accesses

2019-10-17 Thread Sven van Haastregt via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf6248cbb9e7: [OpenCL] Preserve addrspace in CGClass 
(PR43145) (authored by svenvh).

Changed prior to commit:
  https://reviews.llvm.org/D68403?vs=223037=225426#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68403

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -11,6 +11,7 @@
 
 void foo() {
   D d;
+  //CHECK-LABEL: foo
   //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
   //CHECK: call spir_func i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
   d.getmb();
@@ -20,3 +21,32 @@
 
 //CHECK: define linkonce_odr spir_func i32 @_ZNU3AS41D5getmbEv(%class.D 
addrspace(4)* %this)
 //CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
+
+
+// Calling base method through multiple inheritance.
+
+class B2 {
+  public:
+void baseMethod() const {  }
+int bb;
+};
+
+class Derived : public B, public B2 {
+  public:
+void work() const { baseMethod(); }
+// CHECK-LABEL: work
+// CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+};
+
+void pr43145(const Derived *argDerived) {
+  argDerived->work();
+}
+
+// Casting from base to derived.
+
+void pr43145_2(B *argB) {
+  Derived *x = (Derived*)argB;
+}
+
+// CHECK-LABEL: @_Z9pr43145_2
+// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -246,7 +246,8 @@
 
   // Apply the base offset.
   llvm::Value *ptr = addr.getPointer();
-  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8PtrTy);
+  unsigned AddrSpace = ptr->getType()->getPointerAddressSpace();
+  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8Ty->getPointerTo(AddrSpace));
   ptr = CGF.Builder.CreateInBoundsGEP(ptr, baseOffset, "add.ptr");
 
   // If we have a virtual component, the alignment of the result will
@@ -381,7 +382,9 @@
 
   QualType DerivedTy =
 getContext().getCanonicalType(getContext().getTagDeclType(Derived));
-  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo();
+  unsigned AddrSpace =
+BaseAddr.getPointer()->getType()->getPointerAddressSpace();
+  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo(AddrSpace);
 
   llvm::Value *NonVirtualOffset =
 CGM.GetNonVirtualBaseClassOffset(Derived, PathBegin, PathEnd);


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -11,6 +11,7 @@
 
 void foo() {
   D d;
+  //CHECK-LABEL: foo
   //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
   //CHECK: call spir_func i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
   d.getmb();
@@ -20,3 +21,32 @@
 
 //CHECK: define linkonce_odr spir_func i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* %this)
 //CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
+
+
+// Calling base method through multiple inheritance.
+
+class B2 {
+  public:
+void baseMethod() const {  }
+int bb;
+};
+
+class Derived : public B, public B2 {
+  public:
+void work() const { baseMethod(); }
+// CHECK-LABEL: work
+// CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+};
+
+void pr43145(const Derived *argDerived) {
+  argDerived->work();
+}
+
+// Casting from base to derived.
+
+void pr43145_2(B *argB) {
+  Derived *x = (Derived*)argB;
+}
+
+// CHECK-LABEL: @_Z9pr43145_2
+// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -246,7 +246,8 @@
 
   // Apply the base offset.
   llvm::Value *ptr = addr.getPointer();
-  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8PtrTy);
+  unsigned AddrSpace = ptr->getType()->getPointerAddressSpace();
+  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8Ty->getPointerTo(AddrSpace));
   ptr = CGF.Builder.CreateInBoundsGEP(ptr, baseOffset, "add.ptr");
 
   // If we have a virtual component, the alignment of the result will
@@ -381,7 +382,9 @@
 
   QualType DerivedTy =
 getContext().getCanonicalType(getContext().getTagDeclType(Derived));
-  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo();
+  unsigned AddrSpace =
+BaseAddr.getPointer()->getType()->getPointerAddressSpace();
+  

[PATCH] D68981: [clangd] Use our own relation kind.

2019-10-17 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8e3f43ab514: [clangd] Use our own relation kind. (authored 
by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68981

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Relation.cpp
  clang-tools-extra/clangd/index/Relation.h
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  llvm/include/llvm/ADT/DenseMapInfo.h

Index: llvm/include/llvm/ADT/DenseMapInfo.h
===
--- llvm/include/llvm/ADT/DenseMapInfo.h
+++ llvm/include/llvm/ADT/DenseMapInfo.h
@@ -67,6 +67,17 @@
   }
 };
 
+// Provide DenseMapInfo for unsigned chars.
+template <> struct DenseMapInfo {
+  static inline unsigned char getEmptyKey() { return ~0; }
+  static inline unsigned char getTombstoneKey() { return ~0 - 1; }
+  static unsigned getHashValue(const unsigned char ) { return Val * 37U; }
+
+  static bool isEqual(const unsigned char , const unsigned char ) {
+return LHS == RHS;
+  }
+};
+
 // Provide DenseMapInfo for unsigned shorts.
 template <> struct DenseMapInfo {
   static inline unsigned short getEmptyKey() { return 0x; }
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -482,7 +482,7 @@
   std::vector Result;
   RelationsRequest Req;
   Req.Subjects.insert(Subject);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   Index->relations(Req,
[](const SymbolID , const Symbol ) {
  Result.push_back(Object.ID);
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -673,8 +673,7 @@
   const Symbol  = findSymbol(Symbols, "Base");
   const Symbol  = findSymbol(Symbols, "Derived");
   EXPECT_THAT(Relations,
-  Contains(Relation{Base.ID, index::SymbolRole::RelationBaseOf,
-Derived.ID}));
+  Contains(Relation{Base.ID, RelationKind::BaseOf, Derived.ID}));
 }
 
 TEST_F(SymbolCollectorTest, References) {
Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -152,9 +152,9 @@
   SymbolID Base = cantFail(SymbolID::fromStr("6481EE7AF2841756"));
   SymbolID Derived = cantFail(SymbolID::fromStr("6512AEC512EA3A2D"));
   ASSERT_TRUE(bool(ParsedYAML->Relations));
-  EXPECT_THAT(*ParsedYAML->Relations,
-  UnorderedElementsAre(
-  Relation{Base, index::SymbolRole::RelationBaseOf, Derived}));
+  EXPECT_THAT(
+  *ParsedYAML->Relations,
+  UnorderedElementsAre(Relation{Base, RelationKind::BaseOf, Derived}));
 }
 
 std::vector YAMLFromSymbols(const SymbolSlab ) {
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -83,20 +83,15 @@
   SymbolID D{"D"};
 
   RelationSlab::Builder Builder;
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B});
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, C});
-  Builder.insert(Relation{B, index::SymbolRole::RelationBaseOf, D});
-  Builder.insert(Relation{C, index::SymbolRole::RelationBaseOf, D});
-  Builder.insert(Relation{B, index::SymbolRole::RelationChildOf, A});
-  Builder.insert(Relation{C, index::SymbolRole::RelationChildOf, A});
-  Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, B});
-  Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, C});
+  

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: lld/ELF/Driver.cpp:515
+
+  if (llvm::timeTraceProfilerEnabled()) {
+SmallString<128> path(config->outputFile);

Could you possibly move this block (and the init block above) into a new file 
in `lld/Common/` so we can use it in the COFF driver as well?



Comment at: lld/ELF/Driver.cpp:527
+  }
+  return;
 }

Extraneous return.



Comment at: lld/ELF/Options.td:361
 
+def time_trace: F<"time-trace">, HelpText<"Record time trace">;
+

Given this option is a candidate for the other LLD drivers, I am wondering if 
we couldn't have a shared `lld/Common/CommonOpt.td`. @ruiu WDYT?



Comment at: llvm/lib/Support/TimeProfiler.cpp:70
   void begin(std::string Name, llvm::function_ref Detail) {
+std::lock_guard Lock(Mu);
 Stack.emplace_back(steady_clock::now(), TimePointType(), std::move(Name),

The lock is definitely not the way to go, it would skew all the timings 
especially on tens-of-cores machines. Like you're suggesting, make 
`TimeTraceProfilerInstance` a TLS, along with a new combiner upon the call to 
`timeTraceProfilerWrite()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69043



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


[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:415
 
 Optional refInDecl(const Decl *D) {
   struct Visitor : ConstDeclVisitor {

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > This should return `SmallVector` now, some 
> > > > > declarations can have both decl and non-decl references.
> > > > Can you give some examples? It seems that non-decl references are 
> > > > handled by other `refIn*` functions.
> > > > 
> > > > ```
> > > > int Foo = OtherBar; // OtherBar is captured at the momment.
> > > > ```
> > > ```
> > > namespace $1[[a]] = $2[[std]];
> > > ```
> > > 
> > > `$1` is a declaration of a namespace alias.
> > > `$2` is a reference to namespace std.
> > I was just about to make the same point. After this patch lands I would 
> > like to detect `using namespace`s inside function body, and currently there 
> > is no way to distinguish between a usingdirective and a namespacealias. 
> > Since both is only referencing the target namespace.
> > 
> > So even if this starts reporting `$2` in the comment above, there should be 
> > a difference between that and `$3` in `using namespace $3[[std]];`
> That's intentional, though. When we report references, we deliberately leave 
> out information on where it's coming from - otherwise it's hard to encode it.
> 
> However, if we really need that, we could also pass a `NamedDecl*` when 
> reporting declarations (that would mean a slightly different interface, 
> though).
> 
> We still have some complexity budget in `findExplicitReferences` and we 
> should probably spend it here.
> OTOH, adding more stuff will probably bloat it too much, I hope we'll stop 
> there... There will still be cases when one would have to write a separate 
> AST traversal to do more complicated stuff. It's unfortunate that this is 
> twice as expensive, but it shouldn't matter in most cases (e.g. when running 
> inside `Tweak::apply`)
> 
> I'd suggest we keep this patch with `IsDecl` flag, but happy to look into 
> your use-case with `using namespaces` more closely...
as discussed offline, it is a special enough use-case to handle in 
define-inline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977



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


r375118 - [OpenCL] Preserve addrspace in CGClass (PR43145)

2019-10-17 Thread Sven van Haastregt via cfe-commits
Author: svenvh
Date: Thu Oct 17 07:12:51 2019
New Revision: 375118

URL: http://llvm.org/viewvc/llvm-project?rev=375118=rev
Log:
[OpenCL] Preserve addrspace in CGClass (PR43145)

PR43145 revealed two places where Clang was attempting to create a
bitcast without considering the address space of class types during
C++ class code generation.

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

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=375118=375117=375118=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Oct 17 07:12:51 2019
@@ -246,7 +246,8 @@ ApplyNonVirtualAndVirtualOffset(CodeGenF
 
   // Apply the base offset.
   llvm::Value *ptr = addr.getPointer();
-  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8PtrTy);
+  unsigned AddrSpace = ptr->getType()->getPointerAddressSpace();
+  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8Ty->getPointerTo(AddrSpace));
   ptr = CGF.Builder.CreateInBoundsGEP(ptr, baseOffset, "add.ptr");
 
   // If we have a virtual component, the alignment of the result will
@@ -381,7 +382,9 @@ CodeGenFunction::GetAddressOfDerivedClas
 
   QualType DerivedTy =
 getContext().getCanonicalType(getContext().getTagDeclType(Derived));
-  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo();
+  unsigned AddrSpace =
+BaseAddr.getPointer()->getType()->getPointerAddressSpace();
+  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo(AddrSpace);
 
   llvm::Value *NonVirtualOffset =
 CGM.GetNonVirtualBaseClassOffset(Derived, PathBegin, PathEnd);

Modified: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl?rev=375118=375117=375118=diff
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl (original)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl Thu Oct 17 
07:12:51 2019
@@ -11,6 +11,7 @@ public:
 
 void foo() {
   D d;
+  //CHECK-LABEL: foo
   //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
   //CHECK: call spir_func i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
   d.getmb();
@@ -20,3 +21,32 @@ void foo() {
 
 //CHECK: define linkonce_odr spir_func i32 @_ZNU3AS41D5getmbEv(%class.D 
addrspace(4)* %this)
 //CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
+
+
+// Calling base method through multiple inheritance.
+
+class B2 {
+  public:
+void baseMethod() const {  }
+int bb;
+};
+
+class Derived : public B, public B2 {
+  public:
+void work() const { baseMethod(); }
+// CHECK-LABEL: work
+// CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+};
+
+void pr43145(const Derived *argDerived) {
+  argDerived->work();
+}
+
+// Casting from base to derived.
+
+void pr43145_2(B *argB) {
+  Derived *x = (Derived*)argB;
+}
+
+// CHECK-LABEL: @_Z9pr43145_2
+// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*


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


[clang-tools-extra] r375117 - [clangd] Use our own relation kind.

2019-10-17 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Thu Oct 17 07:08:28 2019
New Revision: 375117

URL: http://llvm.org/viewvc/llvm-project?rev=375117=rev
Log:
[clangd] Use our own relation kind.

Summary:
Move the RelationKind from Serialization.h to Relation.h. This patch doesn't
introduce any breaking changes.

Reviewers: kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/MemIndex.h
clang-tools-extra/trunk/clangd/index/Relation.cpp
clang-tools-extra/trunk/clangd/index/Relation.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.h
clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/trunk/clangd/unittests/DexTests.cpp
clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp
clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=375117=375116=375117=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Oct 17 07:08:28 2019
@@ -18,6 +18,7 @@
 #include "URI.h"
 #include "index/Index.h"
 #include "index/Merge.h"
+#include "index/Relation.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
 #include "clang/AST/ASTContext.h"
@@ -1107,7 +1108,7 @@ static void fillSubTypes(const SymbolID
  const SymbolIndex *Index, int Levels, PathRef TUPath) 
{
   RelationsRequest Req;
   Req.Subjects.insert(ID);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   Index->relations(Req, [&](const SymbolID , const Symbol ) {
 if (Optional ChildSym =
 symbolToTypeHierarchyItem(Object, Index, TUPath)) {

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=375117=375116=375117=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Oct 17 07:08:28 2019
@@ -75,7 +75,7 @@ struct RefsRequest {
 
 struct RelationsRequest {
   llvm::DenseSet Subjects;
-  index::SymbolRole Predicate;
+  RelationKind Predicate;
   /// If set, limit the number of relations returned from the index.
   llvm::Optional Limit;
 };

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=375117=375116=375117=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Thu Oct 17 07:08:28 2019
@@ -92,7 +92,8 @@ void MemIndex::relations(
   Req.Limit.getValueOr(std::numeric_limits::max());
   for (const SymbolID  : Req.Subjects) {
 LookupRequest LookupReq;
-auto It = Relations.find(std::make_pair(Subject, Req.Predicate));
+auto It = Relations.find(
+std::make_pair(Subject, static_cast(Req.Predicate)));
 if (It != Relations.end()) {
   for (const auto  : It->second) {
 if (Remaining > 0) {

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.h?rev=375117=375116=375117=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.h Thu Oct 17 07:08:28 2019
@@ -27,8 +27,9 @@ public:
 for (const std::pair>  : Refs)
   this->Refs.try_emplace(R.first, R.second.begin(), R.second.end());
 for (const Relation  : Relations)
-  this->Relations[std::make_pair(R.Subject, R.Predicate)].push_back(
-  R.Object);
+  this->Relations[std::make_pair(R.Subject,
+ static_cast(R.Predicate))]
+  .push_back(R.Object);
   }
   // Symbols are owned by BackingData, Index takes ownership.
   

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-17 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

> I can't wait for @hans next Windows Snapshot, this is my gate for rolling 
> into our builds and CI system, after that, I'm gonna catch every single 
> person who tries to build/check in unclang-formatted.

As it happens, there's a new snapshot out today :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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


[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Mostly NITs from my side, the change LG, thanks!




Comment at: clang-tools-extra/clangd/FindTarget.cpp:422
+  // "using namespace" declaration doesn't have a name.
+  Refs.push_back({D->getQualifierLoc(),
+  D->getIdentLocation(),

Same comment as below: could you say `ReferenceLoc` explicitly here and in 
other places? 
Plain initializer lists are somewhat hard to parse



Comment at: clang-tools-extra/clangd/FindTarget.cpp:436
+  // For namespace alias, "namespace Foo = Target;", we add two references.
+  VisitNamedDecl(D); // add a declaration reference for Foo.
+  // Add a non-declaration reference for Target.

NIT: maybe put this comment on a previous line?
Should read more naturally.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:658
+} else if (auto *E = N.get()) {
+  if (auto Ref = refInExpr(E))
+return {*Ref};

I'd probably suggest to return `SmallVector` from all 
functions, so that they compose better in this situation.

Although `Optional` captures the semantics of some of those 
better, I don't think it's particularly useful at this point.
And having to deconstruct all optionals here leads to a lot of clunky code, 
it'd be much better if we could keep the structure the same as it's used to be.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:661
+} else if (auto *NNSL = N.get()) {
+  return {{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+   explicitReferenceTargets(

Could we have `{ReferenceLoc{` instead of `{{`?
Plain initializer lists are hard to comprehend, especially when they're nested.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:667
+return {*Ref};
+  // return refInTypeLoc(*TL);
+} else if (const CXXCtorInitializer *CCI = N.get()) {

Remove this comment? Seems to be a leftover from experiments



Comment at: clang-tools-extra/clangd/FindTarget.cpp:670
   if (CCI->isBaseInitializer())
-return refInTypeLoc(CCI->getBaseClassLoc());
+if (auto Ref = refInTypeLoc(CCI->getBaseClassLoc()))
+  return {*Ref};

NIT: add braces around an inner multi-line if?



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2280
 }
-EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls));
+EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls))
+<< File.code();

This looks unrelated. Maybe revert and land separately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977



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


[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

2019-10-17 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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


[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2019-10-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

This actually depends on another Diff : https://reviews.llvm.org/D32478
That other change introduces the ability to "unindent operator", which is 
required here; however, it also changes AlignOperands to have more than 2 
modes, which does not seem to be acceptable.

Before merge, should I thus merge "unindent operator" ability back in this 
change? Or create another commit for that part only, to be reviewed separately?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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


[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 225420.
hokein marked 6 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2277,7 +2277,8 @@
   if (const auto *ND = llvm::dyn_cast(D))
 Names.push_back(ND->getQualifiedNameAsString());
 }
-EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls));
+EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls))
+<< File.code();
   }
 }
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -600,59 +600,67 @@
   }
 )cpp",
"0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   "1: targets = {ns::global}, qualifier = 'ns::', decl\n"},
   // Simple types.
   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
-   $0^Struct x;
-   $1^Typedef y;
-   static_cast<$2^Struct*>(0);
+   $0^Struct $1^x;
+   $2^Typedef $3^y;
+   static_cast<$4^Struct*>(0);
  }
)cpp",
"0: targets = {Struct}\n"
-   "1: targets = {Typedef}\n"
-   "2: targets = {Struct}\n"},
+   "1: targets = {x}, decl\n"
+   "2: targets = {Typedef}\n"
+   "3: targets = {y}, decl\n"
+   "4: targets = {Struct}\n"},
   // Name qualifiers.
   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  void foo() {
-   $0^a::$1^b::$2^S x;
-   using namespace $3^a::$4^b;
-   $5^S::$6^type y;
+   $0^a::$1^b::$2^S $3^x;
+   using namespace $4^a::$5^b;
+   $6^S::$7^type $8^y;
  }
 )cpp",
"0: targets = {a}\n"
"1: targets = {a::b}, qualifier = 'a::'\n"
"2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-   "3: targets = {a}\n"
-   "4: targets = {a::b}, qualifier = 'a::'\n"
-   "5: targets = {a::b::S}\n"
-   "6: targets = {a::b::S::type}, qualifier = 'struct S::'\n"},
+   "3: targets = {x}, decl\n"
+   "4: targets = {a}\n"
+   "5: targets = {a::b}, qualifier = 'a::'\n"
+   "6: targets = {a::b::S}\n"
+   "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
+   "8: targets = {y}, decl\n"},
   // Simple templates.
   {R"cpp(
   template  struct vector { using value_type = T; };
   template <> struct vector { using value_type = bool; };
   void foo() {
-$0^vector vi;
-$1^vector vb;
+$0^vector $1^vi;
+$2^vector $3^vb;
   }
 )cpp",
"0: targets = {vector}\n"
-   "1: targets = {vector}\n"},
+   "1: targets = {vi}, decl\n"
+   "2: targets = {vector}\n"
+   "3: targets = {vb}, decl\n"},
   // Template type aliases.
   {R"cpp(
 template  struct vector { using value_type = T; };
 template <> struct vector { using value_type = bool; };
 template  using valias = vector;
 void foo() {
-  $0^valias vi;
-  $1^valias vb;
+  $0^valias $1^vi;
+  $2^valias $3^vb;
 }
   )cpp",
"0: targets = {valias}\n"
-   "1: targets = {valias}\n"},
+   "1: targets = {vi}, decl\n"
+   "2: targets = {valias}\n"
+   "3: targets = {vb}, decl\n"},
   // MemberExpr should know their using declaration.
   {R"cpp(
 struct X { void func(int); }
@@ -694,13 +702,14 @@
 };
 
 void foo() {
-  for (int x : $0^vector()) {
-$1^x = 10;
+  for (int $0^x : $1^vector()) {
+$2^x = 10;
   }
 }
 )cpp",
-   "0: targets = {vector}\n"
-   "1: targets = {x}\n"},
+   "0: targets = {x}, decl\n"
+   "1: targets = {vector}\n"
+   "2: targets = {x}\n"},
   // Handle UnresolvedLookupExpr.
   

[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

2019-10-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:850
+   "1: targets = {Foo::Foo}, decl\n"
+   "2: targets = {Foo::~Foo}, decl\n"
+   "3: targets = {Foo}\n"

ilya-biryukov wrote:
> Could we avoid reporting destructor references for now? I feel they would 
> create more confusion than bring value.
> Mostly worried that they overlap with a reference to the type name and now 
> all client code will probably want to filter out destructor references.
> 
> I believe they're not critical to any of the use-cases we have so far, having 
> a ``// FIXME: decide how to surface destructors when we need them` should be 
> good enough for now
Agree. it is not needed in rename feature as well, we have a typeLoc occurrence 
in `~[[Foo]]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977



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


[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment.

In D69043#1712542 , @ruiu wrote:

> This seems useful. Can I see an example output?


Thanks.

Here's an example from Building clang with ThinLTO: F10296420: clang-10.json 


Using linker flags: -Wl,-time-trace -Wl,-time-trace-granularity=5

I increased the granularity from the default, otherwise the trace gets very 
large (~800MiB).

> Speaking of the output file, I'd make --time-trace option to take an output 
> filename, as an output executable doesn't always have an extension (On 
> Windows it's almost always .exe, but on Unix, extension is usually omitted).

Okay. I'll look at doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69043



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


[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

2019-10-17 Thread Luís Marques via Phabricator via cfe-commits
luismarques added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:97
   RISCVABI::ABI getTargetABI() const { return TargetABI; }
+  bool isRegisterReservedByUser(size_t i) const {
+return UserReservedRegister[i];

This should take a `Register` argument instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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


  1   2   >