[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71241#1787265 , @hfinkel wrote:

> In D71241#1786959 , @jdoerfert wrote:
>
> > In D71241#1786530 , @ABataev wrote:
> >
> > > Most probably, we can use this solution without adding a new expression. 
> > > `DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being 
> > > used. You can use FoundDecl to point to the original function and used 
> > > decl to point to the function being called in this context. But at first, 
> > > we need to be sure that we can handle all corner cases correctly.
> >
> >
> > What new expression are you talking about?
>
>
> To be clear, I believe he's talking about the new expression that I proposed 
> we add in order to represent this kind of call. If that's not needed, and we 
> can use the FoundDecl/Decl pair for that purpose, that should also be 
> considered.


So far, I haven't seen a reason why we would need any new expression. If you 
think we need one, please explain to me why.

FWIW. The only open question I have is with the OpenMP committee, I'll have to 
figure out if we really want to restrict the variant selection to calls only. 
Anyway, we can implement it either way in this scheme.

If I get to it tomorrow, I'll split the patch and update it based on my newest 
version. I'll also write test cases for all the situations we get wrong right 
now (https://reviews.llvm.org/D71241#1782650).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D71588: [objc_direct] fix uniquing when re-declaring a readwrite-direct property

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: arphaman, rjmccall, dexonsmith.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.
MadCoder added a reviewer: liuliu.

ObjCMethodDecl::getCanonicalDecl() for re-declared readwrite properties,
only looks in the ObjCInterface for the declaration of the setter
method, which it won't find.

When the method is a property accessor, we must look in extensions for a
possible redeclaration.

Extend the CG test to cover this issue.

Radar-Id: rdar://problem/57991337


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71588

Files:
  clang/lib/AST/DeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo 
getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD = Ext->getMethod(getSelector(),
+  isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD = Ext->getMethod(getSelector(),
+  isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71455: [NFC] Fix typos in Clangd and Clang

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Renaming `handleDeclOccurence`, `handleMacroOccurence` and  
`handleModuleOccurence` are definitely not NFC because they are public APIs 
that can be used by downstream projects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71455



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1786959 , @jdoerfert wrote:

> In D71241#1786530 , @ABataev wrote:
>
> > Most probably, we can use this solution without adding a new expression. 
> > `DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being used. 
> > You can use FoundDecl to point to the original function and used decl to 
> > point to the function being called in this context. But at first, we need 
> > to be sure that we can handle all corner cases correctly.
>
>
> What new expression are you talking about?


To be clear, I believe he's talking about the new expression that I proposed we 
add in order to represent this kind of call. If that's not needed, and we can 
use the FoundDecl/Decl pair for that purpose, that should also be considered.

> This solution already does point to both declarations, as shown here: 
> https://reviews.llvm.org/D71241#1782504


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D70157: Align branches within 32-Byte boundary

[annita.zhang via Phabricator via cfe-commits]

annita.zhang added a comment.

In D70157#1787139 , @reames wrote:

> Here are the minutes from our phone call a few minutes ago.


Thanks for coordinating the meeting and having a clear summary. It helps a lot 
to accelerate the patch review. I really appreciate it!

> Annita will refresh current patch with two key changes.  1) Drop prefix 
> support and simplify and 2) drop clang driver support for now.  Desire is to 
> minimize cycle time before next iteration so that feedback on approach can be 
> given while reviewers are still around.

Yes, we are working on it right now. Hopefully we can submit a new patch today 
or tomorrow.

> Philip will prototype directive parsing support.  Annita and Yuo (??) to 
> handle coordination on syntax.

I suppose it's Annita and Fangrui

> Side note to Annita: For you to remove "hard code", you'll have to have a 
> placeholder for the enable/disable interface.  That should probably be split 
> and rebased in my patch.

Let's do it in your directive patch.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

[Greg Parker via Phabricator via cfe-commits]

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

Good. `REQUIRES-ANY` was preserved when the boolean expressions were added 
because libc++ was still using it. libc++ has since changed their tests so 
removing it should be fine now.
(previous discussion: D18185 )

The risk for any remaining users of `REQUIRES-ANY` is that they will see a test 
run too often. That's an acceptable failure mode (unlike silently not running 
the test at all).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408



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

[PATCH] D71541: [Clang]: fix spelling mistake in assert message

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

Could you add "[NFC]" in the title. 
That means No Function Change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71541



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

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

This patch is only for RISCV.
The title should have prefix "[RISCV]".




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:585
+const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) {
+  // -mabi is not encoded in bitcode so we need to pass it to LTO code

Argument list should be aligned.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.h:31
+const llvm::opt::ArgList ,
+llvm::opt::ArgStringList );
 } // end namespace riscv

Argument list should be aligned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387



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

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:60
+  if (const auto *MD = dyn_cast_or_null(Parent))
+return true;
+}

Use `isa` rather than `dyn_cast_or_null`.  Furthermore, the `dyn_cast_or_null` 
is wrong - `Parent` is guaranteed to be !NULL given the condition on L56.

```
if (const auto *VD = dyn_cast(ND))
  if (const auto *P = VD->getParentFunctionOrMethod())
if (isa(P) || isa(P))
  return true;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71301



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

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@Jim Thanks, I'll move it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66839



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

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

[kamlesh kumar via Phabricator via cfe-commits]

kamleshbhalui added a comment.

In D71508#1786799 , @aprantl wrote:

> Are we sure we want to canonicalize *before* applying -fdebug-prefix-map in 
> `remapDIPath()`? Honest question, the answer could be yes :-)


it canonicalizes before apply -fdebug-prefix-map in `remapDIPath()`  only when 
creating compilation unit with filename passed by driver  option -main-file-name
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L615

Other than that it first apply -fdebug-prefix-map in `remapDIPath()` then try 
to canonicalize.
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L441


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

[kamlesh kumar via Phabricator via cfe-commits]

kamleshbhalui added a comment.

In D71508#1786804 , @dblaikie wrote:

> In D71508#1786212 , @kamleshbhalui 
> wrote:
>
> > In D71508#1786148 , @probinson 
> > wrote:
> >
> > > In D71508#1786122 , 
> > > @kamleshbhalui wrote:
> > >
> > > > In D71508#1785767 , @probinson 
> > > > wrote:
> > > >
> > > > > Do we have a similar problem if the filespec has an embedded ./ or 
> > > > > ../ in it?
> > > >
> > > >
> > > > problems  occur only when filespec starts with ./ otherwise it's fine.
> > >
> > >
> > > So do other kinds of paths get canonicalized elsewhere?  I'm thinking if 
> > > there's already a place that undoes embedded .. then it should handle 
> > > leading . as well.
> >
> >
> > other canonicalization happens here 
> >  
> > https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L541
> >  and 
> >  
> > https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L441
> >
> > but that does not undo any embedded . and .. anywhere,except leading ./
> >
> > i.e. when we pass like
> >  $clang .././p1.c -S -emit-llvm -g
> >
> > IRGen gives single file entry like this
> >  !1 = !DIFile(filename: ".././p1.c"
> >
> > As you can see path is  not canonicalized but it atleast does not have 
> > duplicate file entry.
>
>
> Even if that .c file is referred to by a different path - what about a 
> #include of that .c file (with some #ifdefs to avoid infinite include 
> recursion) by the absolute path, for instance? does that still not end up 
> with duplicate files with two different paths to the same file?


No, that does not happen. Because in that case we create file entry for first 
time with absolute path and later we always get the same file with 
getOrCreateFile.

The duplicate File entry only happens with the file passed by driver 
-main-file-name p1.cc which used  in creating compilation unit. 
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L518

and while creating debuginfo for variable / function it refers to file by 
quering getOrCreateFile with relative path specified on commmand line and it 
sees that there is no such entry and ends up creating new file entry with this 
path hence duplication.
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L406

> 
> 
>> 
>> 
>>> Is a leading .. okay or a problem?
>> 
>> yes It's ok in the sense ,it does not create duplicate file entry in debug 
>> info.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

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

rsmith added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2522
+  if (CGM.getTriple().isOSWindows())
+return llvm::GlobalValue::LinkOnceODRLinkage;
   return llvm::GlobalValue::WeakODRLinkage;

I think the thread wrapper should probably be `linkonce_odr` across all 
platforms, at least in all TUs that don't contain a definition of the variable. 
Every such TU is supposed to provide its own copy regardless, so making it 
non-discardable seems to serve no purpose.

That said, I suspect this is only hiding the real problem (by discarding the 
symbol before it creates a link error), and you'd still get link errors if you 
have two TUs that both use the same thread-local variable and happen to not 
inline / discard their thread wrappers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

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

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



Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

rsmith wrote:
> rnk wrote:
> > Is this C++11 comment still relevant? I assume that `isComplete` handles 
> > this case by returning true, and a forward decl can tell us if the enum is 
> > scoped.
> I don't think this is useful -- and probably nor is the `isComplete` check. 
> I'll look into this in a follow-up commit.
Fixed in rGeea8ba097c4a86632b88291bea51eb710f8ae4fb. Turns out this was hiding 
a bug in how we checked `static_cast(...)` for incomplete enum types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71576



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

[PATCH] D71586: [clangd] Add a test case that verifies -fimplicit-modules work in Clangd when building the AST

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: sammccall, jkorous.
Herald added subscribers: usaxena95, ributzka, kadircet, dexonsmith, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This patch adds a test case that verifies that `-fmodules -fimplicit-modules` 
work in Clangd when building the AST for Objective-C sources.

I based it on the test case attached to 
https://github.com/clangd/clangd/issues/198. I'm still looking into 
https://github.com/clangd/clangd/issues/198, but it might be unrelated to 
`-fimplicit-modules` support.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71586

Files:
  clang-tools-extra/clangd/test/Inputs/implicit-modules/MyModule.h
  clang-tools-extra/clangd/test/Inputs/implicit-modules/compile_commands.json
  clang-tools-extra/clangd/test/Inputs/implicit-modules/hdr.h
  clang-tools-extra/clangd/test/Inputs/implicit-modules/implicit-modules.m
  clang-tools-extra/clangd/test/Inputs/implicit-modules/module.modulemap
  clang-tools-extra/clangd/test/Inputs/implicit-modules/open.jsonrpc
  clang-tools-extra/clangd/test/implicit-modules.test

Index: clang-tools-extra/clangd/test/implicit-modules.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implicit-modules.test
@@ -0,0 +1,11 @@
+# We need to splice paths into file:// URIs for this test.
+# UNSUPPORTED: windows-msvc
+
+# Use a copy of inputs, as we'll mutate it (as will the background index).
+# RUN: rm -rf %t
+# RUN: cp -r %S/Inputs/implicit-modules %t
+# Need to embed the correct temp path in the actual JSON-RPC requests.
+# RUN: sed -i -e "s|DIRECTORY|%t|" %t/open.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
+
+# RUN: clangd -lit-test < %t/open.jsonrpc | FileCheck %t/open.jsonrpc
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/open.jsonrpc
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/open.jsonrpc
@@ -0,0 +1,52 @@
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+"processId": 123,
+"rootPath": "clangd",
+"capabilities": {},
+"trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://DIRECTORY/implicit-modules.m",
+  "languageId": "objective-c",
+  "version": 1,
+  "text": "@import MyModule;\n\nint main(){\nvoid *_ = foo(1, 2);\nreturn foo(1, 2);\n}"
+}
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "id": 1,
+  "method": "sync",
+  "params": null
+}
+# CHECK: "diagnostics": [
+# CHECK: {
+# CHECK: "code": "-Wint-conversion",
+# CHECK: "message": "Incompatible integer to pointer conversion initializing 'void *' with an expression of type 'char'",
+# CHECK: "range": {
+# CHECK:   "end": {
+# CHECK: "character": 7,
+# CHECK: "line": 3
+# CHECK:   },
+# CHECK:   "start": {
+# CHECK: "character": 6,
+# CHECK: "line": 3
+# CHECK:   }
+# CHECK: },
+# CHECK: "severity": 2,
+# CHECK: "source": "clang"
+# CHECK: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/module.modulemap
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/module.modulemap
@@ -0,0 +1,6 @@
+module MyModule {
+umbrella header "MyModule.h"
+
+export *
+module * { export * }
+}
\ No newline at end of file
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/implicit-modules.m
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/implicit-modules.m
@@ -0,0 +1,6 @@
+@import MyModule;
+
+int main() {
+  void *_ = foo(1, 2);
+  return foo(1, 2);
+}
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/hdr.h
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/hdr.h
@@ -0,0 +1 @@
+char foo(int x, int y);
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/compile_commands.json
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/compile_commands.json
@@ -0,0 +1,5 @@
+[{
+  "directory": "DIRECTORY",
+  "command": "clang -c implicit-modules.m -I DIRECTORY -fmodules -fimplicit-modules",
+  "file": "DIRECTORY/implicit-modules.m"
+}]
Index: clang-tools-extra/clangd/test/Inputs/implicit-modules/MyModule.h
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/implicit-modules/MyModule.h
@@ -0,0 +1 @@
+#import "hdr.h"
___
cfe-commits mailing 

[clang] eea8ba0 - Check whether the destination is a complete type in a static_cast (or

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-16T18:33:35-08:00
New Revision: eea8ba097c4a86632b88291bea51eb710f8ae4fb

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

LOG: Check whether the destination is a complete type in a static_cast (or
C-style cast) to an enumeration type.

We previously forgot to check this, and happened to get away with it
(with bad diagnostics) only because we misclassified incomplete
enumeration types as not being unscoped enumeration types. This also
fixes the misclassification.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaCast.cpp
clang/test/SemaCXX/enum.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2529f7f7f727..3ffbbed17444 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6679,7 +6679,7 @@ def err_upcast_to_inaccessible_base : Error<
 def err_bad_dynamic_cast_not_ref_or_ptr : Error<
   "invalid target type %0 for dynamic_cast; target type must be a reference or 
pointer type to a defined class">;
 def err_bad_dynamic_cast_not_class : Error<"%0 is not a class type">;
-def err_bad_dynamic_cast_incomplete : Error<"%0 is an incomplete type">;
+def err_bad_cast_incomplete : Error<"%0 is an incomplete type">;
 def err_bad_dynamic_cast_not_ptr : Error<"cannot use dynamic_cast to convert 
from %0 to %1">;
 def err_bad_dynamic_cast_not_polymorphic : Error<"%0 is not polymorphic">;
 

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 85447d682ba2..c5ad711d872e 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -1860,12 +1860,8 @@ bool Type::isIntegralOrUnscopedEnumerationType() const {
 }
 
 bool Type::isUnscopedEnumerationType() const {
-  // Check for a complete enum type; incomplete enum types are not properly an
-  // enumeration type in the sense required here.
-  // C++0x: However, if the underlying type of the enum is fixed, it is
-  // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))
-return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+return !ET->getDecl()->isScoped();
 
   return false;
 }

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 741cf6387606..d0b9fe122895 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -740,7 +740,7 @@ void CastOperation::CheckDynamicCast() {
 assert(DestPointer && "Reference to void is not possible");
   } else if (DestRecord) {
 if (Self.RequireCompleteType(OpRange.getBegin(), DestPointee,
- diag::err_bad_dynamic_cast_incomplete,
+ diag::err_bad_cast_incomplete,
  DestRange)) {
   SrcExpr = ExprError();
   return;
@@ -785,7 +785,7 @@ void CastOperation::CheckDynamicCast() {
   const RecordType *SrcRecord = SrcPointee->getAs();
   if (SrcRecord) {
 if (Self.RequireCompleteType(OpRange.getBegin(), SrcPointee,
- diag::err_bad_dynamic_cast_incomplete,
+ diag::err_bad_cast_incomplete,
  SrcExpr.get())) {
   SrcExpr = ExprError();
   return;
@@ -1182,6 +1182,11 @@ static TryCastResult TryStaticCast(Sema , 
ExprResult ,
   // The same goes for reverse floating point promotion/conversion and
   // floating-integral conversions. Again, only floating->enum is relevant.
   if (DestType->isEnumeralType()) {
+if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
+ diag::err_bad_cast_incomplete)) {
+  SrcExpr = ExprError();
+  return TC_Failed;
+}
 if (SrcType->isIntegralOrEnumerationType()) {
   Kind = CK_IntegralCast;
   return TC_Success;
@@ -1651,7 +1656,7 @@ TryStaticImplicitCast(Sema , ExprResult , 
QualType DestType,
   CastKind , bool ListInitialization) {
   if (DestType->isRecordType()) {
 if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
- diag::err_bad_dynamic_cast_incomplete) ||
+ diag::err_bad_cast_incomplete) ||
 Self.RequireNonAbstractType(OpRange.getBegin(), DestType,
 diag::err_allocation_of_abstract_type)) {
   msg = 0;

diff  --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp
index 16ebebe31b6d..b193a17ebf9e 100644
--- a/clang/test/SemaCXX/enum.cpp
+++ b/clang/test/SemaCXX/enum.cpp
@@ -88,19 +88,13 @@ enum { }; // expected-warning{{declaration does not declare 
anything}}
 typedef enum { }; // 

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D70157#1787139 , @reames wrote:

> Here are the minutes from our phone call a few minutes ago.
>
> Attendees: Andy Kaylor, Craig Topper, Annita Zhang, Tom Stellard, Chandler 
> Carruth, Fedor Sergeev, Philip Reames, Yuanake Luo


Thanks for organization the meeting and making the summary.

> Stawman syntax proposal
> 
> .align_branch_boundary disable/default
>  .align_branch_boundary enable N, instructions (fused, jcc, jmp, etc..)
> 
> ...
> 
> Push/Pop semantics were suggested at one point, but were thought to be 
> non-idiomatic?

There is a precedant: .pushsection/.popsection (MCStreamer::SectionStack). With 
.push_align_branch/.pop_align_branch, we probably don't need the 
'switch-to-default' action.

I don't know how likely we may ever need nested states (e.g. an `.include` 
directive inside an .align_branch region where the included file has own idea 
about branch alignment), but .push/.pop does not seem to be more complex than 
disable/enable/default.

I confirm that the following 4 commits have been to the binutils-gdb repository 
(https://sourceware.org/ml/binutils/2019-12/msg00138.html 
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=e379e5f385f874adb0b414f917adb1fc50e20de9).

  gas: Add md_generic_table_relax_frag
  i386: Align branches within a fixed boundary
  i386: Add -mbranches-within-32B-boundaries
  i386: Add tests for -malign-branch-boundary and -malign-branch


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

https://reviews.llvm.org/D70157



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

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

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

rsmith closed this revision.
rsmith marked 4 inline comments as done.
rsmith added a comment.

Committed as rG4b0029995853fe37d1dc95ef96f46697c743fcad 
.




Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

rnk wrote:
> Is this C++11 comment still relevant? I assume that `isComplete` handles this 
> case by returning true, and a forward decl can tell us if the enum is scoped.
I don't think this is useful -- and probably nor is the `isComplete` check. 
I'll look into this in a follow-up commit.



Comment at: clang/test/SemaCXX/warn-enum-compare.cpp:79
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with 
different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two 
values with different enumeration types ('name1::Baz' and 'name2::Baz')}}

rnk wrote:
> It seems more technically correct to say that two values are being compared, 
> but I don't see how to keep the diagnostic as well factored as you have it.
Yeah, I agonized over this a little, but while I agree with you, in the end I 
think it's not entirely wrong to talk about (eg) a comparing an int with a 
float, and the ambiguity between types and values there doesn't seem likely to 
actually be confusing -- I don't think the extra words help comprehension of 
the diagnostic. I'm happy to change it back, of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71576



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

[clang] 4b00299 - [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-16T17:49:45-08:00
New Revision: 4b0029995853fe37d1dc95ef96f46697c743fcad

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

LOG: [c++20] Add deprecation warnings for the expression forms deprecated by 
P1120R0.

This covers:
 * usual arithmetic conversions (comparisons, arithmetic, conditionals)
   between different enumeration types
 * usual arithmetic conversions between enums and floating-point types
 * comparisons between two operands of array type

The deprecation warnings are on-by-default (in C++20 compilations); it
seems likely that these forms will become ill-formed in C++23, so
warning on them now by default seems wise.

For the first two bullets, off-by-default warnings were also added for
all the cases where we didn't already have warnings (covering language
modes prior to C++20). These warnings are in subgroups of the existing
-Wenum-conversion (except that the first case is not warned on if either
enumeration type is anonymous, consistent with our existing
-Wenum-conversion warnings).

Added: 
clang/test/CXX/expr/expr.arith.conv/p2.cpp

Modified: 
clang/include/clang/AST/Type.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/Sema/switch.c
clang/test/Sema/warn-conditional-emum-types-mismatch.c
clang/test/SemaCXX/deprecated.cpp
clang/test/SemaCXX/self-comparison.cpp
clang/test/SemaCXX/warn-enum-compare.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 882b5947c9f4..25ab44c4bcbe 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1973,6 +1973,7 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 
   /// Determine whether this type is an integral or unscoped enumeration type.
   bool isIntegralOrUnscopedEnumerationType() const;
+  bool isUnscopedEnumerationType() const;
 
   /// Floating point categories.
   bool isRealFloatingType() const; // C99 6.2.5p10 (float, double, long double)

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 6cb1a4e0700d..8aa93d4138a2 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,7 +60,27 @@ def UndefinedBoolConversion : 
DiagGroup<"undefined-bool-conversion">;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
-def EnumConversion : DiagGroup<"enum-conversion">;
+def DeprecatedEnumCompareConditional :
+  DiagGroup<"deprecated-enum-compare-conditional">;
+def EnumCompareConditional : DiagGroup<"enum-compare-conditional",
+   [DeprecatedEnumCompareConditional]>;
+def EnumCompareSwitch : DiagGroup<"enum-compare-switch">;
+def DeprecatedEnumCompare : DiagGroup<"deprecated-enum-compare">;
+def EnumCompare : DiagGroup<"enum-compare", [EnumCompareSwitch,
+ DeprecatedEnumCompare]>;
+def DeprecatedAnonEnumEnumConversion : 
DiagGroup<"deprecated-anon-enum-enum-conversion">;
+def DeprecatedEnumEnumConversion : 
DiagGroup<"deprecated-enum-enum-conversion">;
+def DeprecatedEnumFloatConversion : 
DiagGroup<"deprecated-enum-float-conversion">;
+def AnonEnumEnumConversion : DiagGroup<"anon-enum-enum-conversion",
+   [DeprecatedAnonEnumEnumConversion]>;
+def EnumEnumConversion : DiagGroup<"enum-enum-conversion",
+   [DeprecatedEnumEnumConversion]>;
+def EnumFloatConversion : DiagGroup<"enum-float-conversion",
+[DeprecatedEnumFloatConversion]>;
+def EnumConversion : DiagGroup<"enum-conversion",
+   [EnumEnumConversion,
+EnumFloatConversion,
+EnumCompareConditional]>;
 def ObjCSignedCharBoolImplicitIntConversion :
   DiagGroup<"objc-signed-char-bool-implicit-int-conversion">;
 def ImplicitIntConversion : DiagGroup<"implicit-int-conversion",
@@ -126,6 +146,7 @@ def FinalDtorNonFinalClass : 
DiagGroup<"final-dtor-non-final-class">;
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;
 
+def DeprecatedArrayCompare : DiagGroup<"deprecated-array-compare">;
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : 

[clang] 4e9f137 - If constant evaluation fails due to an unspecified pointer comparison,

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-16T17:49:45-08:00
New Revision: 4e9f1379b9cd7ddce8cf182707e976ebceb72b05

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

LOG: If constant evaluation fails due to an unspecified pointer comparison,
produce a note saying that rather than the default "evaluation failed"
note.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/CXX/expr/expr.const/p2-0x.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 814e1e61bd74..25f11213ba55 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -70,6 +70,8 @@ def note_constexpr_pointer_subtraction_not_same_array : Note<
   "subtracted pointers are not elements of the same array">;
 def note_constexpr_pointer_subtraction_zero_size : Note<
   "subtraction of pointers to type %0 of zero size">;
+def note_constexpr_pointer_comparison_unspecified : Note<
+  "comparison has unspecified value">;
 def note_constexpr_pointer_comparison_base_classes : Note<
   "comparison of addresses of subobjects of 
diff erent base classes "
   "has unspecified value">;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f3856f58b113..9b6d0cb59b0b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -11625,8 +11625,10 @@ EvaluateComparisonBinaryOperator(EvalInfo , const 
BinaryOperator *E,
 if (!HasSameBase(LHSValue, RHSValue)) {
   // Inequalities and subtractions between unrelated pointers have
   // unspecified or undefined behavior.
-  if (!IsEquality)
-return Error(E);
+  if (!IsEquality) {
+Info.FFDiag(E, diag::note_constexpr_pointer_comparison_unspecified);
+return false;
+  }
   // A constant address may compare equal to the address of a symbol.
   // The one exception is that address of an object cannot compare equal
   // to a null pointer constant.

diff  --git a/clang/test/CXX/expr/expr.const/p2-0x.cpp 
b/clang/test/CXX/expr/expr.const/p2-0x.cpp
index e84753759c90..cc6380e044fb 100644
--- a/clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ b/clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -472,22 +472,22 @@ namespace UnspecifiedRelations {
   // 
diff erent objects that are not members of the same array or to 
diff erent
   // functions, or if only one of them is null, the results of pq, p<=q,
   // and p>=q are unspecified.
-  constexpr bool u1 = p < q; // expected-error {{constant expression}}
-  constexpr bool u2 = p > q; // expected-error {{constant expression}}
-  constexpr bool u3 = p <= q; // expected-error {{constant expression}}
-  constexpr bool u4 = p >= q; // expected-error {{constant expression}}
-  constexpr bool u5 = p < (int*)0; // expected-error {{constant expression}}
-  constexpr bool u6 = p <= (int*)0; // expected-error {{constant expression}}
-  constexpr bool u7 = p > (int*)0; // expected-error {{constant expression}}
-  constexpr bool u8 = p >= (int*)0; // expected-error {{constant expression}}
-  constexpr bool u9 = (int*)0 < q; // expected-error {{constant expression}}
-  constexpr bool u10 = (int*)0 <= q; // expected-error {{constant expression}}
-  constexpr bool u11 = (int*)0 > q; // expected-error {{constant expression}}
-  constexpr bool u12 = (int*)0 >= q; // expected-error {{constant expression}}
+  constexpr bool u1 = p < q; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u2 = p > q; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u3 = p <= q; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u4 = p >= q; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u5 = p < (int*)0; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u6 = p <= (int*)0; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u7 = p > (int*)0; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u8 = p >= (int*)0; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u9 = (int*)0 < q; // expected-error {{constant expression}} 
expected-note {{comparison has unspecified value}}
+  constexpr bool u10 = 

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

Here are the minutes from our phone call a few minutes ago.

Attendees: Andy Kaylor, Craig Topper, Annita Zhang, Tom Stellard, Chandler 
Carruth, Fedor Sergeev, Philip Reames, Yuanake Luo

Status Summary
==

Performance data has been posted to llvm-dev.  We had a side discussion about 
nop encoding, and it was mentioned these numbers were collected from runs 
targeting skylake (i.e. not generic x86).  This is similar to the result we 
(Azul) have collected and shared summaries of previously.

GNU patch has landed Friday - this mostly fixes assembler command line.

Discussion on Approach
==

Three major options debated:

  Assembler only - as in the current patch, assembler does all work, only 
command line flag
  Explicit Directive only - as proposed in my alternate patch, compiler decides 
exactly what instructions get aligned
  Region based directives - as proposed in James' last comment on review, 
directives enable and disable auto-padding in assembler

Use cases identified:

  compiler users
  important than assembler is self contained (i.e. don't have to know 
compiler options for reproduceability)
  inline assembly looks a lot like assembler users
  legacy assembler
  important that existing assembly works unmodified
  assembler users
  "try it and see" model vs selective enable vs selective disable
  likely need to support all three

Consensus was that the region based directives met use cases the best.  In 
particular, desire to be able to overrule default (for say, inline assembly or 
a JITs patchability assumptions) and then restore default.  Default assembler 
behavior remains unchanged.

Stawman syntax proposal

.align_branch_boundary disable/default
.align_branch_boundary enable N, instructions (fused, jcc, jmp, etc..)

We need to ensure a consensus on syntax is shared w/gnu.  Annita agreed to 
coordinate this.

Compiler would essentially just wrap generated assembly in directives.

Issue noticed while writing this up: proposed syntax assumes a default has been 
set, but doesn't give a way to set one.  This would seem to break the desired 
reproducibility property for compiled code.  Revision needed.

Push/Pop semantics were suggested at one point, but were thought to be 
non-idiomatic?

Other Topics


We very quickly discussed nop vs prefix performance.  There was a clear 
consensus in starting with nop only patch and evolving later as needed.

Next Steps
==

Annita will refresh current patch with two key changes.  1) Drop prefix support 
and simplify and 2) drop clang driver support for now.  Desire is to minimize 
cycle time before next iteration so that feedback on approach can be given 
while reviewers are still around.

Philip will prototype directive parsing support.  Annita and Yuo (??) to handle 
coordination on syntax.

Suggested patch split:

  (current patch) command line option to set default, nop only version 
w/cleaned up code as much as possible
  assembler directive support (draft by Philip in parallel)
  (future) compiler patch to wrap by default

Side note to Annita: For you to remove "hard code", you'll have to have a 
placeholder for the enable/disable interface.  That should probably be split 
and rebased in my patch.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

[Xin-Xin Wang via Phabricator via cfe-commits]

xinxinw1 created this revision.
xinxinw1 added reviewers: beanz, phosek, xiaobai, smeenai.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With %p, each test file that we're using to generate profile data will make its 
own profraw file which is around 60 MB in size. If we have a lot of test files, 
that quickly uses a lot of space. Use %4m instead to share the profraw files 
used to store the profile data. We use 4 here based on the default value in 
https://reviews.llvm.org/source/llvm-github/browse/master/llvm/CMakeLists.txt$604


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71585

Files:
  clang/utils/perf-training/lit.cfg


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, 
sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70157: Align branches within 32-Byte boundary

[Kan Shengchen via Phabricator via cfe-commits]

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



Comment at: llvm/lib/MC/MCFragment.cpp:426
+  case MCFragment::FT_MachineDependent: {
+const MCMachineDependentFragment *MF =
+cast(this);

MaskRay wrote:
> `const auto *`. The type is obvious according to the right hand side.
Shall we keep consistent with the local code style?  `const MCLEBFragment *LF = 
cast(this);` was used here.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.



Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want

vsk wrote:
> delcypher wrote:
> > The compiler-rt codebase tends to use `SANITIZER_MAC` macro (defined to be 
> > 1 if Apple otherwise it's 0) rather than `__APPLE__`.
> I see. That seems problematic, as it makes it tricky to add macOS-specific 
> (or iOS-specific) functionality down the road. I don't think SANITIZER_MAC 
> should be defined unless TARGET_OS_MACOS is true.
Sorry I should clarify. `SANITIZER_MAC` is poorly named but it is defined to be 
`1` for Apple platforms and `0`. I'm just pointing out the convention that 
exists today. You're absolutely right that we might want to do different things 
for different Apple platforms but I don't think we want to start doing a mass 
re-name until arm64e and arm64_32 support are completed landed in llvm's master.


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

https://reviews.llvm.org/D71491



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

[PATCH] D71566: New checks for fortified sprintf

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Thanks for working on this! I think this would be a really useful diagnostic to 
have.




Comment at: clang/lib/Sema/SemaChecking.cpp:311
 }
+class EstimateSizeFormatHandler
+: public analyze_format_string::FormatStringHandler {

nit: `namespace {`



Comment at: clang/lib/Sema/SemaChecking.cpp:370
   // FIXME: There are some more useful checks we could be doing here:
   //  - Analyze the format string of sprintf to see how much of buffer is used.
   //  - Evaluate strlen of strcpy arguments, use as object size.

Can you delete this comment now?



Comment at: clang/lib/Sema/SemaChecking.cpp:383
   bool IsChkVariant = false;
+  llvm::APSInt UsedSize = llvm::APSInt::get(-1);
   unsigned SizeIndex, ObjectIndex;

`Optional` might be a better way of representing this?



Comment at: clang/lib/Sema/SemaChecking.cpp:388
 return;
+  case Builtin::BI__builtin___sprintf_chk: {
+if (auto *StrE = dyn_cast(

Can't you do this for `Builtin::sprintf` as well? The diagnostic would still be 
worth issuing when `_FORTIFY_SOURCE` is disabled.



Comment at: clang/lib/Sema/SemaChecking.cpp:392
+  EstimateSizeFormatHandler H(StrE);
+  StringRef StrRef = StrE->getString();
+  const char *Str = StrRef.data();

Will this assert on: `sprintf(buf, L"foo");`? Not that that makes any sense, 
but we shouldn't crash.



Comment at: clang/lib/Sema/SemaChecking.cpp:399
+  size_t StrLen =
+  std::min(std::max(TypeSize, size_t(1)) - 1, StrRef.size());
+  if (!analyze_format_string::ParsePrintfString(

I'm not sure why you're using `min` here, can you add a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566



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

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71475#1784312 , @cchen wrote:

> In D71475#1784284 , @jdoerfert wrote:
>
> > What is the output when you run the example with `k` in `lastprivate` or 
> > `reduction`?
>
>
> I actually got the same result (return value) changing from firstprivate to 
> lastprivate. Not so sure how to make linear work with reduction.


I don't want it to "work" but I thought we should give a proper error message. 
Can you add test cases where a value is used in the step *and* in a (1) shared, 
(2) private, (3) firstprivate, ...?

Other than that I think the code changes look pretty good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Steven Wu via Phabricator via cfe-commits]

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

SGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

[David Blaikie via Phabricator via cfe-commits]

dblaikie added subscribers: JDevlieghere, aprantl.
dblaikie added a comment.

Yeah, GCC's debug info for this is... not good. I don't think it's anything 
we'd need to be inspired by. Here's the tags and names:

  DW_TAG_compile_unit
DW_AT_name   ("templ2.cpp")
DW_TAG_structure_type
  DW_AT_name ("type")
DW_TAG_variable
  DW_AT_name ("variable")
DW_TAG_base_type
  DW_AT_name ("int")
DW_TAG_variable
  DW_AT_name ("variable")
DW_TAG_base_type
  DW_AT_name ("float")
DW_TAG_subprogram
  DW_AT_name ("main")
  DW_TAG_variable
DW_AT_name   ("t")
  DW_TAG_variable
DW_AT_name   ("i")
  DW_TAG_variable
DW_AT_name   ("f")

The structure_type has no child DIEs and no mention of the variable template 
instantiations - the global variables are both called "global" but with 
different types. No mention of the scope they were actually declared in. I 
guess maybe at least this "works" for free in some ways, but probably makes it 
impossible to refer to one or the other variable or with the right name.

@aprantl @probinson @echristo @JDevlieghere - So my proposal is that the 
'right' DWARF for this is:

  DW_TAG_structure_type
DW_AT_name ("type")
DW_TAG_member
  DW_AT_name ("variable")
  DW_AT_declaration (true)
DW_TAG_member
  DW_AT_name ("variable")
  DW_AT_declaration (true)
  DW_TAG_variable
DW_AT_specification (0x "variable")
DW_AT_location
DW_AT_linkage_name ("_ZN4type1iE")
  DW_TAG_base_type
DW_AT_name ("int")
  DW_TAG_variable
DW_AT_specification (0x "variable")
DW_AT_location
DW_AT_linkage_name ("_ZN4type1fE")
  DW_TAG_base_type
DW_AT_name ("float")

And the LLVM IR should be done the same way as member function templates - 
never appear in the member list, but appear otherwise the same - a 
DIGlobalVariable named "variable" for the definition with a 'declaration:' 
attribute that refers to a DIDerivedType DW_TAG_member for the declaration of 
"variable" where the 'scope:' attribute of the DW_TAG_member refers to the 
DW_TAG_structure_type, but that structure_type's member list does not contain 
this member. (same as functions - DISubprogram definition -declaration-> 
DISubprogram declaration -scope-> DW_TAG_structure_type but doesn't appear in 
the member list of the structure_type)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70537



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2530
+  return "ios";
+// FIXME: Add macCatalyst support here ("\"mac catalyst\"").
+llvm_unreachable("macCatalyst isn't yet supported");

steven_wu wrote:
> arphaman wrote:
> > steven_wu wrote:
> > > Why not support that in this commit? I don't see problem returning a 
> > > string with space inside.
> > The rest of macCatalyst support is not yet upstreamed (specifically the new 
> > `Environment` value)
> Doesn't this mean you hit llvm_unreachable when you link catalyst? And this 
> is the default behavior?
You can't link for macCatalyst with the current `master` Clang. I will modify 
the downstream Clang to pass in the right flag though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy )
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, ,
-  ),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  , ),

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > Do we really need to set `HasMapper` to `true` unconditionally here and 
> > > in other places?
> > For `map`, `to`, and `from` clauses, they can have mappers, so it's set to 
> > `true`. For `is_device_ptr` and `use_device_ptr`, it's set to `false`.
> So, it completely depends on the clause kind, right? If so, can we just 
> remove it and rely on the clause kind?
This is used in `OMPMappableExprListClause` which is inherited by all these 
clauses. Do you mean to check the template type there to get the clause kind?


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

https://reviews.llvm.org/D67833



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2530
+  return "ios";
+// FIXME: Add macCatalyst support here ("\"mac catalyst\"").
+llvm_unreachable("macCatalyst isn't yet supported");

arphaman wrote:
> steven_wu wrote:
> > Why not support that in this commit? I don't see problem returning a string 
> > with space inside.
> The rest of macCatalyst support is not yet upstreamed (specifically the new 
> `Environment` value)
Doesn't this mean you hit llvm_unreachable when you link catalyst? And this is 
the default behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Thanks! I made some comments. This code is convoluted. I don't think I can 
reason through all the edge cases, but this seems like an improvement. I'd 
suggest adding the recommended tests and any other corner case inputs you can 
think of, and after another round of review we can land it.




Comment at: clang/test/CodeGen/ms-inline-asm-64.c:18
 // CHECK: call void asm sideeffect inteldialect
-// CHECK-SAME: mov [eax], $0
+// CHECK-SAME: mov qword ptr [eax], $0
 // CHECK-SAME: "r,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}})

I guess this has the same semantics as it did before.



Comment at: llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h:72
+  : NeedBracs(false), Imm(0), BaseReg(StringRef()), IndexReg(StringRef()),
+OffsetName(StringRef()), Scale(1) {}
+  // [BaseReg + IndexReg * ScaleExpression + OFFSET name + ImmediateExpression]

These explicit empty StringRef initializations seem unnecessary, I'd suggest 
removing them.



Comment at: llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h:97
   SMLoc Loc;
   unsigned Len;
   int64_t Val;

nit, pack the bool after the unsigned for a smaller struct. Hey, we make a 
vector of these. :)



Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:5935-5940
+auto rewrite_it = it;
+while (++rewrite_it != AsmStrRewrites.end() &&
+   rewrite_it->Loc.getPointer() <= AR.IntelExp.OffsetName.data()) {
+  if (OffsetName.data() == rewrite_it->Loc.getPointer() &&
+  OffsetName.size() == rewrite_it->Len &&
+  rewrite_it->Kind == AOK_Input) {

Maybe this would be simpler with
  auto rewrite_it = find_if(it, AsmStrRewrites.end(), [&](const AsmRewrite 
) {
// ... conditions below
  });



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1481
+InlineAsmIdentifierInfo Info;
+if ((ParseError = ParseIntelOffsetOperator(Val, ID, Info, End)))
+  return true;

While this doesn't trigger warnings, it seems like it would be simpler to do 
the assignment as its own statement and then `if (ParseErrror) return true;`

I see that the local code style uses this pattern already, but I'm not sure 
that's a good reason to promote it and keep using it.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1484
+StringRef ErrMsg;
+if ((ParseError = SM.onOffset(Val, OffsetLoc, ID, Info,
+  isParsingInlineAsm(), ErrMsg)))

ditto



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+getParser().parsePrimaryExpr(Val, End))
+  return Error(Start, "unexpected token!");
+  } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {

Please test this corner case, I imagine it looks like:
  mov eax, offset 3



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+  } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+return Error(Start, "offset operator cannot yet handle constants");
+  }

Please add a test for this corner case, I'm curious to see if the error 
reporting really works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71241#1786530 , @ABataev wrote:

> Most probably, we can use this solution without adding a new expression. 
> `DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being used. 
> You can use FoundDecl to point to the original function and used decl to 
> point to the function being called in this context. But at first, we need to 
> be sure that we can handle all corner cases correctly.


What new expression are you talking about? This solution already does point to 
both declarations, as shown here: https://reviews.llvm.org/D71241#1782504

> For example, can it handle such cases as:
>  t1.c:
> 
>   int hst();
>   #pragma omp declare variant(hst) match(device={kind(host)})
>   int base();
> 
> 
> t2.c:
> 
>   int main() {
> return base();
>   }
> 
> 
> This is the correct C code, I assume. At least, clang compiles it.
> 
> Another problem:
>  t1.c:
> 
>   int hst();
>   #pragma omp declare varint(hst) match(device={kind(host)})
>   int base();
> 
> 
> t2.c:
> 
>   int base() { return 0; }
>   int main() {
> return base();
>   }
> 
> 
> According to the standard, this is valid code and `hst` function must be 
> called (though it is not correct since in C/C++ each definition is a 
> declaration and all restriction applied to the declaration must be applied to 
> the definition too).

The second example is valid code and it doens't matter if the first example is.
The argument I used here https://reviews.llvm.org/D71241#1783711 holds again 
and the `hst` function *must not be called* in either case. 
The standard is clear on that and that is one of the reason the alias solution 
*does not work*.

> Another one possible problem might be with the templates:
> 
>   template  T base() { return T(); }
>   int hst() { return 1; }
>   
>   int main() {
> return base();
>   }
>   
>   #pragma omp declare variant(hst) match(device={kind(gpu)})
>   template
>   T base();
>   
>   int foo() {
> return base();
>   }

It would be helpful if you explain the problem that might or might not arise. 
In the above code there is no `target` so it is unclear if foo would be emitted 
for the gpu or not (main is not emitted for the gpu for sure). Only when foo is 
emitted for the gpu, the call would be to `hst`, otherwise always to `base`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:339
   // Add the deployment target.
-  MachOTC.addMinVersionArgs(Args, CmdArgs);
+  if (!Version[0] || Version[0] >= 520)
+MachOTC.addPlatformVersionArgs(Args, CmdArgs);

steven_wu wrote:
> Why version '0' should go through the new path?
Version `0` indicates that `-mlinker-version=` wasn't specified. We want to use 
the new flag by default.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2530
+  return "ios";
+// FIXME: Add macCatalyst support here ("\"mac catalyst\"").
+llvm_unreachable("macCatalyst isn't yet supported");

steven_wu wrote:
> Why not support that in this commit? I don't see problem returning a string 
> with space inside.
The rest of macCatalyst support is not yet upstreamed (specifically the new 
`Environment` value)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:339
   // Add the deployment target.
-  MachOTC.addMinVersionArgs(Args, CmdArgs);
+  if (!Version[0] || Version[0] >= 520)
+MachOTC.addPlatformVersionArgs(Args, CmdArgs);

Why version '0' should go through the new path?



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2530
+  return "ios";
+// FIXME: Add macCatalyst support here ("\"mac catalyst\"").
+llvm_unreachable("macCatalyst isn't yet supported");

Why not support that in this commit? I don't see problem returning a string 
with space inside.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

Just wanted to say thanks for the performance data! I know it was hard to get, 
but it is really, really useful to help folks evaluate these kinds of changes 
with actual data around the options available.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: steven_wu, dexonsmith.
Herald added subscribers: llvm-commits, ributzka, jkorous.
Herald added projects: clang, LLVM.

In Xcode 11, ld added a new flag called `-platform_version` that can be used 
instead of the old `-_version_min` flags. The new flag allows Clang 
to pass the SDK version from the driver to the linker.
This patch adopts the new `-platform_version` flag in Clang, and starts using 
it by default, unless a linker version < 520 is passed to the driver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71579

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/Inputs/WatchOS6.0.sdk/SDKSettings.json
  clang/test/Driver/Inputs/iPhoneOS13.0.sdk/SDKSettings.json
  clang/test/Driver/darwin-infer-simulator-sdkroot.c
  clang/test/Driver/darwin-ld-platform-version-ios.c
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-ld-platform-version-tvos.c
  clang/test/Driver/darwin-ld-platform-version-watchos.c
  clang/test/Driver/darwin-ld.c
  clang/test/Driver/darwin-sdkroot.c
  clang/test/Driver/target-triple-deployment.c
  llvm/include/llvm/Support/VersionTuple.h

Index: llvm/include/llvm/Support/VersionTuple.h
===
--- llvm/include/llvm/Support/VersionTuple.h
+++ llvm/include/llvm/Support/VersionTuple.h
@@ -87,6 +87,13 @@
 return Build;
   }
 
+  /// Return a version tuple that contains only the first 3 version components.
+  VersionTuple withoutBuild() const {
+if (HasBuild)
+  return VersionTuple(Major, Minor, Subminor);
+return *this;
+  }
+
   /// Determine if two version numbers are equivalent. If not
   /// provided, minor and subminor version numbers are considered to be zero.
   friend bool operator==(const VersionTuple , const VersionTuple ) {
Index: clang/test/Driver/target-triple-deployment.c
===
--- clang/test/Driver/target-triple-deployment.c
+++ clang/test/Driver/target-triple-deployment.c
@@ -1,14 +1,14 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
-// RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
-// RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -mlinker-version=400 -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-darwin9 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.7 -mlinker-version=400 -### %t.o 2>> %t.log
 //
-// RUN: %clang -target armv7-apple-ios -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios0.0 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios1.2.3 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios5.0 -### %t.o 2>> %t.log
-// RUN: %clang -target armv7-apple-ios7.0 -### %t.o 2>> %t.log
-// RUN: %clang -target arm64-apple-ios -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios0.0 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios1.2.3 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios5.0 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target armv7-apple-ios7.0 -mlinker-version=400 -### %t.o 2>> %t.log
+// RUN: %clang -target arm64-apple-ios -mlinker-version=400 -### %t.o 2>> %t.log
 //
 // RUN: FileCheck %s < %t.log
 
Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -43,7 +43,7 @@
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneOS8.0.0.sdk
-// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang -target arm64-apple-darwin --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang -target arm64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-IPHONE %s
 //
 // CHECK-IPHONE: clang
@@ -55,7 +55,7 @@
 //
 // RUN: rm -rf %t/SDKs/iPhoneSimulator8.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneSimulator8.0.sdk
-// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target x86_64-apple-darwin --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
 //
 // CHECK-SIMULATOR: clang
@@ -66,7 +66,7 @@
 //
 // RUN: rm -rf %t/SDKs/MacOSX10.10.0.sdk
 // RUN: mkdir -p %t/SDKs/MacOSX10.10.0.sdk
-// RUN: env SDKROOT=%t/SDKs/MacOSX10.10.0.sdk %clang -target x86_64-apple-darwin --sysroot="" %s -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/MacOSX10.10.0.sdk %clang -target x86_64-apple-darwin -mlinker-version=400 --sysroot="" %s -### 2>&1 \
 

[PATCH] D71503: New warning on for-loops that never run because condition is false on the first iteration

[Richard Trieu via Phabricator via cfe-commits]

rtrieu updated this revision to Diff 234180.

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

https://reviews.llvm.org/D71503

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-loop-analysis-first-condition.cpp

Index: clang/test/SemaCXX/warn-loop-analysis-first-condition.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-loop-analysis-first-condition.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 %s -Wfor-loop-analysis -verify
+// RUN: %clang_cc1 %s -Wall -verify
+
+#define OneHundred 100
+
+void test() {
+  // Loop variable
+  for (int i = OneHundred; i < 50; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 100 on loop initialization, but comparing 100 to 50 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = OneHundred; i > 50; --i) {}
+
+  // Outside variable
+  int j;
+  for (j = OneHundred; j < 50; ++j) {}
+  // expected-warning@-1{{variable 'j' is set to value 100 on loop initialization, but comparing 100 to 50 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (j = OneHundred; j > 50; --j) {}
+
+  // int->float conversion
+  for (int i = OneHundred; i < 5e1; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 100 on loop initialization, but comparing 100 to 5.00e+01 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = OneHundred; i > 5e1; --i) {}
+
+  // const values
+  const int kStart = 0;
+  const int kInterval = 10;
+  for (int i = kStart; i < kStart; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 0 on loop initialization, but comparing 0 to 0 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = kStart; i < kStart + kInterval; ++i) {}
+
+  // Reversed comparison operator
+  for (int i = 100; i <= 1; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 100 on loop initialization, but comparing 100 to 1 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = 100; i >= 1; --i) {}
+
+  // Range endpoints mistakes
+  const int kMin = 0;
+  const int kMax = 8;
+  for (int i = kMin; i < kMin; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 0 on loop initialization, but comparing 0 to 0 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = kMax; i < kMin; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 8 on loop initialization, but comparing 8 to 0 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = kMin; i < kMax; ++i) {}
+
+  for (int i = 16; i < 16; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 16 on loop initialization, but comparing 16 to 16 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = 8; i < 16; ++i) {}
+
+  for (int i = 1; i <= (1 >> 5); ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 1 on loop initialization, but comparing 1 to 0 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = 1; i <= (1 << 5); ++i) {}
+
+  // Complex comparion operand
+  const int TwoK = 1024 * 2;
+  for (int i = 2048; i < TwoK; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 2048 on loop initialization, but comparing 2048 to 2048 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (int i = 1024; i < TwoK; ++i) {}
+
+  // Signedness mistake
+  for (unsigned i = -5; i < 5; ++i) {}
+  // expected-warning@-1{{variable 'i' is set to value 4294967291 on loop initialization, but comparing 4294967291 to 5 in the loop condition is false and the loop body will never run}}
+  // expected-note@-2{{place parentheses around the condition to silence this warning}}
+  for (signed i = -5; i < 5; ++i) {}
+
+  for (int i = 0x; i > 10; i -= 10) {}
+  // expected-warning@-1{{variable 'i' is set to value -1 on loop initialization, but comparing 

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

[Reid Kleckner via Phabricator via cfe-commits]

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

I have no blocking concerns, just some idle thoughts. Up to you if you want 
Aaron's feedback before landing.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6235
+  "%plural{2:with|4:from|:and}0 "
+  "%select{enumeration|floating-point}1 type %3 is deprecated">,
+  InGroup;

I'm surprised we don't have a TableGen facility to say `Warning`, but I don't see an alternative.



Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

Is this C++11 comment still relevant? I assume that `isComplete` handles this 
case by returning true, and a forward decl can tell us if the enum is scoped.



Comment at: clang/test/SemaCXX/warn-enum-compare.cpp:79
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with 
different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two 
values with different enumeration types ('name1::Baz' and 'name2::Baz')}}

It seems more technically correct to say that two values are being compared, 
but I don't see how to keep the diagnostic as well factored as you have it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71576



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

[PATCH] D71503: New warning on for-loops that never run because condition is false on the first iteration

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added a comment.

In D71503#1784697 , @lebedev.ri wrote:

> Thank you for working on this!
>  This seems to be missing tests.


My bad, still getting used to git.  Updated with a test now.


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

https://reviews.llvm.org/D71503



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Haven't looked into too many details yet but made some suggestions anyway...




Comment at: llvm/include/llvm/MC/MCFragment.h:634
+
+  uint64_t getBoundarySize() const {
+return AlignBoundarySize;

Store AlignBoundarySize as a shift value, then `needPadding` doesn't even need 
to call Log2_64().



Comment at: llvm/lib/MC/MCFragment.cpp:426
+  case MCFragment::FT_MachineDependent: {
+const MCMachineDependentFragment *MF =
+cast(this);

`const auto *`. The type is obvious according to the right hand side.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:104
+  else if (BranchType == "indirect")
+addKind(AlignBranchIndirect);
+}

An unknown value is just ignored, e.g. `--x86-align-branch=unknown`. I think 
there should be an error, but I haven't looked into the patch detail to 
confidently suggest how we should surface this error.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:185
+  AlignMaxPrefixSize = 5;
+} else {
+  assert((X86AlignBranchBoundary == 0 || (X86AlignBranchBoundary >= 32 &&

> } else {

Should --x86-branches-within-32B-boundaries overwrite 
--x86-align-branch-boundary and --x86-align-branch and 
--x86-align-branch-prefix-size? My feeling is that it just provides a default 
value if either of the three options is not specified.

If you are going to remove `addKind` calls here, you can delete this member 
function.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:196
+}
+if(AlignBoundarySize != 0) {
+  AlignBoundarySize = Align(AlignBoundarySize).value();

space after if

No curly braces around simple statements.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:660
+  for (auto *F = CF; F && F != LastBranch; F = F->getPrevNode()) {
+bool IsMF = F->getKind() == MCFragment::FT_MachineDependent;
+// If there is a fragment that neither has instructions nor is

`isa(F)`




Comment at: llvm/lib/Target/X86/X86InstrInfo.td:1020
 def X86_COND_B   : PatLeaf<(i8 2)>;  // alt. COND_C
-def X86_COND_AE  : PatLeaf<(i8 3)>;  // alt. COND_NC
+def X86_COND_AE  : PatLeaf<(i8 3)>;  // alt. COND_NC,COND_NB
 def X86_COND_E   : PatLeaf<(i8 4)>;  // alt. COND_Z

Unintentional change not part of this patch?



Comment at: llvm/test/MC/X86/align-branch-32-1a.s:1
+# Check the macro-fusion table
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown 
--x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp  
--x86-align-branch-prefix-size=5 %s | llvm-objdump -d - | FileCheck %s

Did an older version include 32-1b.s or 32-1c.s? Now they are missing.



Comment at: llvm/test/MC/X86/align-branch-64-1b.s:1
+# Check only fused conditional jumps and conditional jumps are aligned with 
option --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc  
--x86-align-branch-prefix-size=5
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown 
--x86-align-branch-boundary=32 --x86-align-branch=fused+jcc  
--x86-align-branch-prefix-size=5 %s | llvm-objdump -d  - | FileCheck %s

Create test/MC/X86/Inputs/align-branch-64-1.s and reference it from 1[a-d].s 
via %S/Inputs/align-branch-64-1.s


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

https://reviews.llvm.org/D70157



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

Noting another issue we found in local testing (with an older version of this 
patch).  This interacts badly with the implicit exception mechanism in LLVM.  
For that mechanism, we end up generating assembly which looks more or less like 
this:
Ltmp:

  cmp %rsi, (%rdi)
  jcc 

And a side table which maps TLmp to another label so that a fault at Ltmp can 
be interpreted as an extremely expensive branch via signal handler.

The problem is that the auto-alignment of the fused branch causes padding to be 
introduced which separate the label and the faulting exception, breaking the 
mapping.

Essentially, this comes down to an implicit assumption that the label stays 
tightly bundled with the following instruction.

This can happen with either nop or prefix padding.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

[Sid Manning via Phabricator via cfe-commits]

sidneym updated this revision to Diff 234174.
sidneym added a comment.

Remove quotes.

Also add ld.lld file so that configurations that don't enable lld will still 
pass this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70919

Files:
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/test/Driver/Inputs/hexagon_tree/Tools/bin/ld.lld
  clang/test/Driver/hexagon-toolchain-elf.c


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// 
-
+// Default, not passing -fuse-ld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// 
-
+// Passing -fuse-ld=lld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  -march=
+// CHECK082-NOT:  -mcpu=
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,11 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  const char *Exec = Args.MakeArgString(HTC.GetLinkerPath());
+  bool UseLLD = (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+ llvm::sys::path::stem(Exec).equals_lower("ld.lld"));
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   
//
   // Silence warnings for various options
@@ -232,9 +236,10 @@
   for (const auto  : HTC.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());
 
-  CmdArgs.push_back("-march=hexagon");
-  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
-  CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  if (!UseLLD) {
+CmdArgs.push_back("-march=hexagon");
+CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  }
 
   if (IsShared) {
 CmdArgs.push_back("-shared");


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// -
+// Default, not passing -fuse-ld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// -
+// Passing -fuse-ld=lld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  -march=
+// CHECK082-NOT:  -mcpu=
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,11 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  const char *Exec = Args.MakeArgString(HTC.GetLinkerPath());
+  bool UseLLD = (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+ 

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

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

rsmith created this revision.
rsmith added reviewers: aaron.ballman, rnk.
Herald added a project: clang.

This covers:

- usual arithmetic conversions (comparisons, arithmetic, conditionals) between 
different enumeration types
- usual arithmetic conversions between enums and floating-point types
- comparisons between two operands of array type

The deprecation warnings are on-by-default (in C++20 compilations); it
seems likely that these forms will become ill-formed in C++23, so
warning on them now by default seems wise.

For the first two bullets, off-by-default warnings were also added for
all the cases where we didn't already have warnings (covering language
modes prior to C++20). These warnings are in subgroups of the existing
-Wenum-conversion (except that the first case is not warned on if either
enumeration type is anonymous, consistent with our existing
-Wenum-conversion warnings).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71576

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.arith.conv/p2.cpp
  clang/test/Sema/switch.c
  clang/test/Sema/warn-conditional-emum-types-mismatch.c
  clang/test/SemaCXX/deprecated.cpp
  clang/test/SemaCXX/self-comparison.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -926,18 +926,16 @@
 
   Consistent comparison (operator=)
   https://wg21.link/p0515r3;>P0515R3
-  SVN
+  SVN
 

 https://wg21.link/p0905r1;>P0905R1
   

 https://wg21.link/p1120r0;>P1120R0
-Partial
   

 https://wg21.link/p1185r2;>P1185R2
-SVN
   

 https://wg21.link/p1186r3;>P1186R3
Index: clang/test/SemaCXX/warn-enum-compare.cpp
===
--- clang/test/SemaCXX/warn-enum-compare.cpp
+++ clang/test/SemaCXX/warn-enum-compare.cpp
@@ -76,184 +76,184 @@
   while (td == AnonAA);  // expected-warning {{comparison of constant 'AnonAA' (42) with expression of type 'TD' is always false}}
 #endif
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while (B1 == B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == (name2::B3)); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while B1))) == (((B2; // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while ((name1::B2) == (((name2::B3; // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while z))) == (name2::B2)); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while (x == a); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'name1::Foo')}}
-  while (x == b); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'oneFoo' (aka 'name1::Foo'))}}
-  while (x == c); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'twoFoo' (aka 'name1::Foo'))}}
-
-  while (x == y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x != y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x >= y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x <= y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x > y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x < y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-
-  while (FooB == y); // expected-warning  

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Thanks for the review! Much appreciated :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70579



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

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Yeah, this is working around a more fundamental representational issue.

Like member function templates, member variable templates shouldn't be 
pre-emptively built or ever appear in the "members" list of the type (because 
that list isn't closed - we don't know what instantiations would be needed when 
the type is built so member template instantiations never appear in the member 
list, but instead the type appears as the scope of the member template 
instantiation and are added as member DIEs during DWARF emission lazily (eg: if 
a member function template instantiation gets optimized away, nothing of it 
remains and no member function template declaration of that instantiation will 
appear in as a child of the type DIE))

For instance, it seems creating two instantiations of a member variable 
template seems to get messed up by having a member list with the same 
instantiation twice:

  !9 = !DIDerivedType(tag: DW_TAG_member, name: "variable", scope: !10, file: 
!3, line: 3, baseType: !12, flags: DIFlagStaticMember, extraData: i32 0)
  !10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "type", 
file: !3, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !11, 
identifier: "_ZTS4type")
  !11 = !{!9, !9}

Even though !7 is another member variable template instantiation.

(here's the source:

  struct type {
template 
inline static auto variable = T();
  };
  int main() {
type t;
int i = type::variable;
float f = type::variable;
  }

So clearly something is modifying the "members" list (since it'd be created 
empty after the "type t;" line, then a member variable template for the 
variable would be added to the member list, then when it goes to add 
"variable" it gets messed up and adds another copy of !9)

Oh... both variable and variable definitions share a single 
declaration DIDerivedType... that's no good.

I'm also generally wondering if variable template instantiations should have 
the template parameters in their name, same as function templates - currently 
they don't in LLVM's output at least. Building GCC 9.2 to compare there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70537



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

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D71508#1786212 , @kamleshbhalui 
wrote:

> In D71508#1786148 , @probinson wrote:
>
> > In D71508#1786122 , @kamleshbhalui 
> > wrote:
> >
> > > In D71508#1785767 , @probinson 
> > > wrote:
> > >
> > > > Do we have a similar problem if the filespec has an embedded ./ or ../ 
> > > > in it?
> > >
> > >
> > > problems  occur only when filespec starts with ./ otherwise it's fine.
> >
> >
> > So do other kinds of paths get canonicalized elsewhere?  I'm thinking if 
> > there's already a place that undoes embedded .. then it should handle 
> > leading . as well.
>
>
> other canonicalization happens here 
>  
> https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L541
>  and 
>  
> https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L441
>
> but that does not undo any embedded . and .. anywhere,except leading ./
>
> i.e. when we pass like
>  $clang .././p1.c -S -emit-llvm -g
>
> IRGen gives single file entry like this
>  !1 = !DIFile(filename: ".././p1.c"
>
> As you can see path is  not canonicalized but it atleast does not have 
> duplicate file entry.


Even if that .c file is referred to by a different path - what about a #include 
of that .c file (with some #ifdefs to avoid infinite include recursion) by the 
absolute path, for instance? does that still not end up with duplicate files 
with two different paths to the same file?

> 
> 
>> Is a leading .. okay or a problem?
> 
> yes It's ok in the sense ,it does not create duplicate file entry in debug 
> info.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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

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

[Reid Kleckner via Phabricator via cfe-commits]

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

Looks great to me.

This has the potential to break some existing code, though. I would suggest 
either landing it early in the day, watching for breakage, and hoping for the 
best, or you could try building an application that makes significant use of 
Windows SDK headers to get more confidence that we won't have to revert it. You 
could build the `sbox_integration_tests` target in Chrome or `chrome_elf`, and 
see if that works.




Comment at: clang/lib/AST/ASTContext.cpp:2922
 
+QualType ASTContext::getFunctionTypeWithoutPtrSizes(QualType T) {
+  if (const auto *Proto = T->getAs()) {

Nice, this version is very simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71039



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

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Are we sure we want to canonicalize *before* applying -fdebug-prefix-map in 
`remapDIPath()`? Honest question, the answer could be yes :-)




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:412
 
+  if (!llvm::sys::path::is_absolute(FileName)) {
+  FileName = llvm::sys::path::remove_leading_dotslash(FileName);

I believe that this check is redundant and also part of remove_leading_dotslash?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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

Re: [clang] 00bc76e - Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.

[Vedant Kumar via cfe-commits]

Thanks John, the bot has recovered.

> On Dec 16, 2019, at 1:08 PM, John McCall via cfe-commits 
>  wrote:
> 
> On 16 Dec 2019, at 15:11, John McCall wrote:
>> On 16 Dec 2019, at 15:07, Vedant Kumar wrote:
>>> Hi John,
>>> 
>>> The lldb bot went red after your clang AST changes landed: 
>>> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4801/changes#detail0
>>>  
>>> 
>>> 
>>> All of the test failures were due to a clang assertion:
>>> 
>>> Assertion failed: (isOverloadedOperator() && "Template name isn't an 
>>> overloaded operator?"), function getOperator, file 
>>> /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/include/clang/AST/TemplateName.h,
>>>  line 520.
>>> 
>>> Perhaps this is related to "Commit a9db0d9f - Use property-based 
>>> serialization for TemplateName"?
>> 
>> Sorry, I’ll take a look.
> 
> Should be fixed in 803403afc83f659be1c620eb1896dcf704b18b0a.
> 
> John.
> ___
> 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] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())

mstorsjo wrote:
> rnk wrote:
> > I would say that this is inaccurate. It greatly affects what the optimizer 
> > is allowed to do.
> > 
> > It looks like we forgot to put a comdat on these things, is that not the 
> > correct fix?
> Oh, ok.
> 
> The full case I was trying to fix (but forgot to recheck after changing this 
> bit) is that when used with `-ffunction-sections`, the tls wrapper function 
> ends up as comdat `one_only`, which then gives multiple definition errors. So 
> perhaps the issue is in the handling of `-ffunction-sections` wrt weak_odr?
Ah, that is an interesting wrinkle. I'm surprised that things worked without 
-ffunction-sections, though. I would've expected the two plain external 
definitions to produce multiple definition errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Sam Clegg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a1e349a7933: [WebAssembly] Setting export_name implies 
llvm.used (authored by sbc100).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/wasm-export-name.c


Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo 
to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const 
ParsedAttr ) {


Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0a1e349 - [WebAssembly] Setting export_name implies llvm.used

[Sam Clegg via cfe-commits]

Author: Sam Clegg
Date: 2019-12-16T14:48:38-08:00
New Revision: 0a1e349a7933f7880971533175e11b4bfd22bd53

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

LOG: [WebAssembly] Setting export_name implies llvm.used

This change updates the clang front end to add symbols to llvm.used
when they have explicit export_name attribute.

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/CodeGen/wasm-export-name.c

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index e83688c46be1..678320487453 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@ static void handleWebAssemblyExportNameAttr(Sema , Decl 
*D, const ParsedAttr &
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const 
ParsedAttr ) {

diff  --git a/clang/test/CodeGen/wasm-export-name.c 
b/clang/test/CodeGen/wasm-export-name.c
index b662a272cbac..f2556155dd4c 100644
--- a/clang/test/CodeGen/wasm-export-name.c
+++ b/clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@ int foo(void) {
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo 
to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}



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

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 234160.
sbc100 added a comment.

- revert part


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/wasm-export-name.c


Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo 
to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const 
ParsedAttr ) {


Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

[Brian Gesiak via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG376cf43729c8: [coroutines][PR41909] Generalize fix from 
D62550 (authored by modocache).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70579

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/coroutines.cpp


Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -731,6 +731,12 @@
   [](auto ) -> coro {
 co_await 24;
   }("argument");
+  [](auto ) ->coro { // expected-warning {{expression 
result unused}}
+[]() -> coro {
+  co_await 36;
+};
+co_await 48;
+  };
 }
 template void ok_generic_lambda_coawait_PR41909(); // expected-note {{in 
instantiation of function template specialization 
'ok_generic_lambda_coawait_PR41909' requested here}}
 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -7190,8 +7190,12 @@
   // that may fail.
   ScopeInfo->setNeedsCoroutineSuspends(false);
 
-  // The new CoroutinePromise object needs to be built and put into the current
-  // FunctionScopeInfo before any transformations or rebuilding occurs.
+  // We re-build the coroutine promise object (and the coroutine parameters its
+  // type and constructor depend on) based on the types used in our current
+  // function. We must do so, and set it on the current FunctionScopeInfo,
+  // before attempting to transform the other parts of the coroutine body
+  // statement, such as the implicit suspend statements (because those
+  // statements reference the FunctionScopeInfo::CoroutinePromise).
   if (!SemaRef.buildCoroutineParameterMoves(FD->getLocation()))
 return StmtError();
   auto *Promise = SemaRef.buildCoroutinePromise(FD->getLocation());
@@ -7200,8 +7204,9 @@
   getDerived().transformedLocalDecl(S->getPromiseDecl(), {Promise});
   ScopeInfo->CoroutinePromise = Promise;
 
-  // Transform the implicit coroutine statements we built during the initial
-  // parse.
+  // Transform the implicit coroutine statements constructed using dependent
+  // types during the previous parse: initial and final suspensions, the return
+  // object, and others. We also transform the coroutine function's body.
   StmtResult InitSuspend = getDerived().TransformStmt(S->getInitSuspendStmt());
   if (InitSuspend.isInvalid())
 return StmtError();
@@ -7228,17 +7233,13 @@
 return StmtError();
   Builder.ReturnValue = Res.get();
 
+  // If during the previous parse the coroutine still had a dependent promise
+  // statement, we may need to build some implicit coroutine statements
+  // (such as exception and fallthrough handlers) for the first time.
   if (S->hasDependentPromiseType()) {
-// PR41909: We may find a generic coroutine lambda definition within a
-// template function that is being instantiated. In this case, the lambda
-// will have a dependent promise type, until it is used in an expression
-// that creates an instantiation with a non-dependent promise type. We
-// should not assert or build coroutine dependent statements for such a
-// generic lambda.
-auto *MD = dyn_cast_or_null(FD);
-if (!MD || !MD->getParent()->isGenericLambda()) {
-  assert(!Promise->getType()->isDependentType() &&
- "the promise type must no longer be dependent");
+// We can only build these statements, however, if the current promise type
+// is not dependent.
+if (!Promise->getType()->isDependentType()) {
   assert(!S->getFallthroughHandler() && !S->getExceptionHandler() &&
  !S->getReturnStmtOnAllocFailure() && !S->getDeallocate() &&
  "these nodes should not have been built yet");


Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -731,6 +731,12 @@
   [](auto ) -> coro {
 co_await 24;
   }("argument");
+  [](auto ) ->coro { // expected-warning {{expression result unused}}
+[]() -> coro {
+  co_await 36;
+};
+co_await 48;
+  };
 }
 template void ok_generic_lambda_coawait_PR41909(); // expected-note {{in instantiation of function template specialization 'ok_generic_lambda_coawait_PR41909' requested here}}
 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -7190,8 +7190,12 @@
   // that may fail.
   ScopeInfo->setNeedsCoroutineSuspends(false);
 
-  // The new CoroutinePromise object needs to be built and put into the current
-  // FunctionScopeInfo before any transformations or rebuilding occurs.
+  // We re-build 

[PATCH] D71201: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks. This is good to go!


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

https://reviews.llvm.org/D71201



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

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 234157.
cchen added a comment.

Apply @ABataev's patch, add some code to support the patch and some tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_for_ast_print.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp
  clang/test/OpenMP/target_simd_ast_print.cpp

Index: clang/test/OpenMP/target_simd_ast_print.cpp
===
--- clang/test/OpenMP/target_simd_ast_print.cpp
+++ clang/test/OpenMP/target_simd_ast_print.cpp
@@ -187,6 +187,10 @@
   // CHECK-NEXT: for (T i = 0; i < N; ++i) {
   // CHECK-NEXT: }
 
+#pragma omp target simd linear (i: b + 1)
+  // CHECK: #pragma omp target simd linear(i: b + 1)
+  for (T j = 16; j < 64; j++)
+i += 4;
   return T();
 }
 
@@ -307,6 +311,10 @@
   // CHECK: #pragma omp target simd is_device_ptr(p)
   // CHECK-NEXT: for (int i = 0; i < 2; ++i) {
   // CHECK-NEXT: }
+#pragma omp target simd linear (i: b + 1)
+  // CHECK: #pragma omp target simd linear(i: b + 1)
+  for (int j = 16; j < 64; j++)
+i += 4;
 
   return (tmain(argc, ));
 }
Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,8 @@
 float f;
 char cnt;
 
+int a[100];
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
@@ -255,3 +257,39 @@
 // CHECK: ret void
 #endif
 
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck %s -check-prefix=CK1
+// RUN: %clang_cc1 -DCK1 -fopenmp -x c++ -std=c++11 -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+#ifdef CK1
+
+// CK1: foo
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+// CK1: define internal void [[OUTLINE:@.+]](i32* noalias [[GTID:%.+]], i32* noalias [[BTID:%.+]], i32* dereferenceable(4) [[I_VAR:%.+]], i32* dereferenceable(4) [[K_VAR:%.+]])
+// CK1: [[GTID_ADDR:%.+]] = alloca i32*
+// CK1: [[BTID_ADDR:%.+]] = alloca i32*
+// CK1: [[I_ADDR:%.+]] = alloca i32*
+// CK1: [[K_ADDR:%.+]] = alloca i32*
+// CK1: store i32* [[GTID]], i32** [[GTID_ADDR]]
+// CK1: store i32* [[BTID]], i32** [[BTID_ADDR]]
+// CK1: store i32* [[I_VAR:%.+]], i32** [[I_ADDR]]
+// CK1: store i32* [[K_VAR:%.+]], i32** [[K_ADDR]]
+// CK1: [[ZERO:%.+]] = load i32*, i32** [[I_ADDR]]
+// CK1: [[ONE:%.+]] = load i32*, i32** [[K_ADDR]]
+// CK1: [[TWO:%.+]] = load i32, i32* [[ZERO]]
+// CK1: store i32 [[TWO]], i32* [[LINEAR_START:%.+]]
+// CK1: [[THREE:%.+]] = load i32, i32* [[ONE]]
+// CK1: [[ADD:%.+]] = add nsw i32 [[THREE]]
+// CK1: store i32 [[ADD]], i32* [[LINEAR_STEP:%.+]]
+#endif
Index: clang/test/OpenMP/parallel_for_ast_print.cpp
===
--- clang/test/OpenMP/parallel_for_ast_print.cpp
+++ clang/test/OpenMP/parallel_for_ast_print.cpp
@@ -100,6 +100,11 @@
   // CHECK-NEXT: for (int j = 0; j < 2; ++j)
   // CHECK-NEXT: for (int j = 0; j < 2; ++j)
   // CHECK-NEXT: foo();
+
+  T i;
+#pragma omp parallel for linear (i: b + 1)
+  for (T j = 16; j < 64; j++)
+b += 4;
   return T();
 }
 
@@ -146,6 +151,11 @@
  // CHECK-NEXT: for (int i = 0; i < 10; ++i)
   // CHECK-NEXT: for (int j = 0; j < 10; ++j)
   // CHECK-NEXT: foo();
+
+  int i;
+#pragma omp parallel for linear (i: b + 1)
+  for (int j = 16; j < 64; j++)
+b += 4;
   return (tmain(argc) + tmain(argv[0][0]));
 }
 
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -1124,7 +1124,8 @@
   } else {
 DSAInfo  = getTopOfStack().SharingMap[D];
 assert(Data.Attributes == OMPC_unknown || (A == Data.Attributes) ||
-   (A == OMPC_firstprivate && Data.Attributes == OMPC_lastprivate) ||
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||
(isLoopControlVariable(D).first && A == OMPC_private));
 if (A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) {
@@ -3829,6 +3830,9 @@
   MarkDeclarationsReferencedInExpr(E);
 }
   }
+  if (auto *LC = 

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

> It will still crash for something like `target simd`, need to move the code 
> around a little bit, move it out of `if`.

I tried making `target simd ` crash but it seems work 
(target_simd_ast_print.cpp).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Sam Clegg via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:725
   TOut.emitExportName(WasmSym, ExportName);
+  Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip);
 }

sunfish wrote:
> It feels like this is a little inconsistent. If `export_name` doesn't 
> automatically imply `used` in LLVM IR or .o files, why does it do so in .s 
> files?
OK, reverted that part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493



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

[clang] 376cf43 - [coroutines][PR41909] Generalize fix from D62550

[Brian Gesiak via cfe-commits]

Author: Brian Gesiak
Date: 2019-12-16T17:43:04-05:00
New Revision: 376cf43729c8025eecbd2969522c5687f2a3919f

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

LOG: [coroutines][PR41909] Generalize fix from D62550

Summary:
In https://reviews.llvm.org/D62550 @rsmith pointed out that there are
many situations in which a coroutine body statement may be
transformed/rebuilt as part of a template instantiation, and my naive
check whether the coroutine was a generic lambda was insufficient.

This is indeed true, as I've learned by reading more of the
TreeTransform code. Most transformations are written in a way that
doesn't assume the resulting types are not dependent types. So the
assertion in 'TransformCoroutineBodyStmt', that the promise type must no
longer be dependent, is out of place.

This patch removes the assertion, spruces up some code comments, and
adds a test that would have failed with my naive check from
https://reviews.llvm.org/D62550.

Reviewers: GorNishanov, rsmith, lewissbaker

Reviewed By: rsmith

Subscribers: junparser, EricWF, rsmith, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/coroutines.cpp

Removed: 




diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ade0e6a0c4ce..2e4e91285060 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7190,8 +7190,12 @@ 
TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
   // that may fail.
   ScopeInfo->setNeedsCoroutineSuspends(false);
 
-  // The new CoroutinePromise object needs to be built and put into the current
-  // FunctionScopeInfo before any transformations or rebuilding occurs.
+  // We re-build the coroutine promise object (and the coroutine parameters its
+  // type and constructor depend on) based on the types used in our current
+  // function. We must do so, and set it on the current FunctionScopeInfo,
+  // before attempting to transform the other parts of the coroutine body
+  // statement, such as the implicit suspend statements (because those
+  // statements reference the FunctionScopeInfo::CoroutinePromise).
   if (!SemaRef.buildCoroutineParameterMoves(FD->getLocation()))
 return StmtError();
   auto *Promise = SemaRef.buildCoroutinePromise(FD->getLocation());
@@ -7200,8 +7204,9 @@ 
TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
   getDerived().transformedLocalDecl(S->getPromiseDecl(), {Promise});
   ScopeInfo->CoroutinePromise = Promise;
 
-  // Transform the implicit coroutine statements we built during the initial
-  // parse.
+  // Transform the implicit coroutine statements constructed using dependent
+  // types during the previous parse: initial and final suspensions, the return
+  // object, and others. We also transform the coroutine function's body.
   StmtResult InitSuspend = getDerived().TransformStmt(S->getInitSuspendStmt());
   if (InitSuspend.isInvalid())
 return StmtError();
@@ -7228,17 +7233,13 @@ 
TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
 return StmtError();
   Builder.ReturnValue = Res.get();
 
+  // If during the previous parse the coroutine still had a dependent promise
+  // statement, we may need to build some implicit coroutine statements
+  // (such as exception and fallthrough handlers) for the first time.
   if (S->hasDependentPromiseType()) {
-// PR41909: We may find a generic coroutine lambda definition within a
-// template function that is being instantiated. In this case, the lambda
-// will have a dependent promise type, until it is used in an expression
-// that creates an instantiation with a non-dependent promise type. We
-// should not assert or build coroutine dependent statements for such a
-// generic lambda.
-auto *MD = dyn_cast_or_null(FD);
-if (!MD || !MD->getParent()->isGenericLambda()) {
-  assert(!Promise->getType()->isDependentType() &&
- "the promise type must no longer be dependent");
+// We can only build these statements, however, if the current promise type
+// is not dependent.
+if (!Promise->getType()->isDependentType()) {
   assert(!S->getFallthroughHandler() && !S->getExceptionHandler() &&
  !S->getReturnStmtOnAllocFailure() && !S->getDeallocate() &&
  "these nodes should not have been built yet");

diff  --git a/clang/test/SemaCXX/coroutines.cpp 
b/clang/test/SemaCXX/coroutines.cpp
index 677c6e6ff8d2..9e94fe8c9c10 100644
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/clang/test/SemaCXX/coroutines.cpp
@@ -731,6 +731,12 @@ template void 
ok_generic_lambda_coawait_PR41909() {
   [](auto ) -> coro {
 co_await 24;
   

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

[Martin Storsjö via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())

rnk wrote:
> I would say that this is inaccurate. It greatly affects what the optimizer is 
> allowed to do.
> 
> It looks like we forgot to put a comdat on these things, is that not the 
> correct fix?
Oh, ok.

The full case I was trying to fix (but forgot to recheck after changing this 
bit) is that when used with `-ffunction-sections`, the tls wrapper function 
ends up as comdat `one_only`, which then gives multiple definition errors. So 
perhaps the issue is in the handling of `-ffunction-sections` wrt weak_odr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())

I would say that this is inaccurate. It greatly affects what the optimizer is 
allowed to do.

It looks like we forgot to put a comdat on these things, is that not the 
correct fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Actually, this doesn't seem to be enough for the case at hand (used with 
`-ffunction-sections -fdata-sections`), I'll have to continue looking into it 
tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!


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

https://reviews.llvm.org/D71400



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

[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70527#1785552 , @ikudrin wrote:

> Personally, I would prefer to see the file name and path to be changed as 
> little as possible because that would help to recognize the files better. We 
> cannot use `remove_dots()` on POSIX OSes to simplify paths, because it may 
> return an invalid path; thus we have to use `getRealPath()`. If I understand 
> it right, there is no similar problem with the file name itself.
>
> So, which issues this patch is going to solve?


It seems clear to me, the filename could be an absolute symlink to a real file 
somewhere far removed from the realpath of the parent directory. It seems 
reasonable that -fdiagnostics-absolute-paths would look through symlinks in 
this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527



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

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping


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

https://reviews.llvm.org/D71400



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

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, froydnj, compnerd.
Herald added a project: clang.

On Windows, weak_odr is a no-op, and thus a weak_odr tls wrapper function can 
result in linker errors due to multiply defined symbols. Therefore, use 
linkonce_odr instead of weak_odr for tls wrappers on windows.

This should hopefully fix https://bugzilla.mozilla.org/show_bug.cgi?id=1566288.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71572

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/tls-init-funcs.cpp


Index: clang/test/CodeGenCXX/tls-init-funcs.cpp
===
--- clang/test/CodeGenCXX/tls-init-funcs.cpp
+++ clang/test/CodeGenCXX/tls-init-funcs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.8 -std=c++1y -S -emit-llvm %s 
-o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -std=c++1y -S -emit-llvm %s -o - 
| FileCheck %s --check-prefix=MINGW
 
 // CHECK: @a = internal thread_local global
 // CHECK: @_Z2vtIiE = linkonce_odr thread_local global i32 5
@@ -10,6 +11,9 @@
 // CHECK-DAG: define weak_odr hidden cxx_fast_tlscc i32* @_ZTW2vtIvE()
 // CHECK-DAG: define {{.*}} @_ZTW1a
 
+// MINGW-DAG: define linkonce_odr hidden i32* @_ZTW2vtIiE()
+// MINGW-DAG: define linkonce_odr hidden i32* @_ZTW2vtIvE()
+
 struct A {
   ~A();
 };
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2516,6 +2516,10 @@
 if (!llvm::GlobalVariable::isLinkOnceLinkage(VarLinkage) &&
 !llvm::GlobalVariable::isWeakODRLinkage(VarLinkage))
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())
+return llvm::GlobalValue::LinkOnceODRLinkage;
   return llvm::GlobalValue::WeakODRLinkage;
 }
 


Index: clang/test/CodeGenCXX/tls-init-funcs.cpp
===
--- clang/test/CodeGenCXX/tls-init-funcs.cpp
+++ clang/test/CodeGenCXX/tls-init-funcs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.8 -std=c++1y -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -std=c++1y -S -emit-llvm %s -o - | FileCheck %s --check-prefix=MINGW
 
 // CHECK: @a = internal thread_local global
 // CHECK: @_Z2vtIiE = linkonce_odr thread_local global i32 5
@@ -10,6 +11,9 @@
 // CHECK-DAG: define weak_odr hidden cxx_fast_tlscc i32* @_ZTW2vtIvE()
 // CHECK-DAG: define {{.*}} @_ZTW1a
 
+// MINGW-DAG: define linkonce_odr hidden i32* @_ZTW2vtIiE()
+// MINGW-DAG: define linkonce_odr hidden i32* @_ZTW2vtIvE()
+
 struct A {
   ~A();
 };
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2516,6 +2516,10 @@
 if (!llvm::GlobalVariable::isLinkOnceLinkage(VarLinkage) &&
 !llvm::GlobalVariable::isWeakODRLinkage(VarLinkage))
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())
+return llvm::GlobalValue::LinkOnceODRLinkage;
   return llvm::GlobalValue::WeakODRLinkage;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

From auditing the call sites, it seems that almost all of them could be 
simplified by using the new API:
http://llvm-cs.pcc.me.uk/tools/clang/lib/Basic/FileManager.cpp/rgetCanonicalName




Comment at: clang/include/clang/Basic/FileManager.h:226
   /// The canonical names of directories.
   llvm::DenseMap CanonicalDirNames;
 

You could make the key `void*` and save a DenseMap here. Nobody ever iterates 
CanonicalDirNames to look at the keys.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527



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

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:725
   TOut.emitExportName(WasmSym, ExportName);
+  Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip);
 }

It feels like this is a little inconsistent. If `export_name` doesn't 
automatically imply `used` in LLVM IR or .o files, why does it do so in .s 
files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493



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

[clang] 3f22b47 - Revert "[NFC-I] Remove hack for fp-classification builtins"

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2019-12-16T14:01:51-08:00
New Revision: 3f22b4721e6c9859c392d9891411cbc8d9e10c70

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

LOG: Revert "[NFC-I] Remove hack for fp-classification builtins"

This reverts commit b1e542f302c1ed796ad9f703d4d36e010afcb914.

The original 'hack' didn't chop out fp-16 to double conversions, so
systems that use FP16ConversionIntrinsics end up in IR-CodeGen with an
i16 type isntead of a float type (like PPC64-BE).  The bots noticed
this.

Reverting until I figure out how to fix this

Added: 


Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/crash-invalid-builtin.c

Removed: 
clang/test/Sema/builtin-fpclassification.c



diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 51d3500df8ae..29dec78e4b96 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -399,15 +399,15 @@ BUILTIN(__builtin_islessgreater , "i.", "Fnct")
 BUILTIN(__builtin_isunordered   , "i.", "Fnct")
 
 // Unary FP classification
-BUILTIN(__builtin_fpclassify, "ii.", "Fnct")
-BUILTIN(__builtin_isfinite,   "i.", "Fnct")
-BUILTIN(__builtin_isinf,  "i.", "Fnct")
-BUILTIN(__builtin_isinf_sign, "i.", "Fnct")
-BUILTIN(__builtin_isnan,  "i.", "Fnct")
-BUILTIN(__builtin_isnormal,   "i.", "Fnct")
+BUILTIN(__builtin_fpclassify, "ii.", "Fnc")
+BUILTIN(__builtin_isfinite,   "i.", "Fnc")
+BUILTIN(__builtin_isinf,  "i.", "Fnc")
+BUILTIN(__builtin_isinf_sign, "i.", "Fnc")
+BUILTIN(__builtin_isnan,  "i.", "Fnc")
+BUILTIN(__builtin_isnormal,   "i.", "Fnc")
 
 // FP signbit builtins
-BUILTIN(__builtin_signbit, "i.", "Fnct")
+BUILTIN(__builtin_signbit, "i.", "Fnc")
 BUILTIN(__builtin_signbitf, "if", "Fnc")
 BUILTIN(__builtin_signbitl, "iLd", "Fnc")
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 80d8a486e4cf..aff63aef2934 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5796,35 +5796,51 @@ bool Sema::SemaBuiltinFPClassification(CallExpr 
*TheCall, unsigned NumArgs) {
<< SourceRange(TheCall->getArg(NumArgs)->getBeginLoc(),
   (*(TheCall->arg_end() - 1))->getEndLoc());
 
-  // __builtin_fpclassify is the only case where NumArgs != 1, so we can count
-  // on all preceding parameters just being int.  Try all of those.
-  for (unsigned i = 0; i < NumArgs - 1; ++i) {
-Expr *Arg = TheCall->getArg(i);
-
-if (Arg->isTypeDependent())
-  return false;
-
-ExprResult Res = PerformImplicitConversion(Arg, Context.IntTy, AA_Passing);
-
-if (Res.isInvalid())
-  return true;
-TheCall->setArg(i, Res.get());
-  }
-
   Expr *OrigArg = TheCall->getArg(NumArgs-1);
 
   if (OrigArg->isTypeDependent())
 return false;
 
-  OrigArg = DefaultFunctionArrayLvalueConversion(OrigArg).get();
-  TheCall->setArg(NumArgs - 1, OrigArg);
-
   // This operation requires a non-_Complex floating-point number.
   if (!OrigArg->getType()->isRealFloatingType())
 return Diag(OrigArg->getBeginLoc(),
 diag::err_typecheck_call_invalid_unary_fp)
<< OrigArg->getType() << OrigArg->getSourceRange();
 
+  // If this is an implicit conversion from float -> float, double, or
+  // long double, or half -> half, float, double, or long double, remove it.
+  if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
+// Only remove standard FloatCasts, leaving other casts inplace
+if (Cast->getCastKind() == CK_FloatingCast) {
+  bool IgnoreCast = false;
+  Expr *CastArg = Cast->getSubExpr();
+  if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
+assert(
+(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
+"promotion from float to either float, double, or long double is "
+"the only expected cast here");
+IgnoreCast = true;
+  } else if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Half) 
&&
+ !Context.getTargetInfo().useFP16ConversionIntrinsics()) {
+assert(
+(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Half) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
+"promotion from half to either half, float, double, or long double 
"
+"is the only expected cast here");
+  

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 234144.
sbc100 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/wasm-export-name.c
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/test/MC/WebAssembly/export-name.s


Index: llvm/test/MC/WebAssembly/export-name.s
===
--- llvm/test/MC/WebAssembly/export-name.s
+++ llvm/test/MC/WebAssembly/export-name.s
@@ -22,5 +22,5 @@
 # CHECK-OBJ-NEXT:   - Index:   0
 # CHECK-OBJ-NEXT: Kind:FUNCTION
 # CHECK-OBJ-NEXT: Name:foo
-# CHECK-OBJ-NEXT: Flags:   [ EXPORTED ]
+# CHECK-OBJ-NEXT: Flags:   [ EXPORTED, NO_STRIP ]
 # CHECK-OBJ-NEXT: Function:0
Index: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
===
--- llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -722,6 +722,7 @@
   auto WasmSym = cast(Ctx.getOrCreateSymbol(SymName));
   WasmSym->setExportName(ExportName);
   TOut.emitExportName(WasmSym, ExportName);
+  Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip);
 }
 
 if (DirectiveID.getString() == ".import_module") {
Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo 
to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const 
ParsedAttr ) {


Index: llvm/test/MC/WebAssembly/export-name.s
===
--- llvm/test/MC/WebAssembly/export-name.s
+++ llvm/test/MC/WebAssembly/export-name.s
@@ -22,5 +22,5 @@
 # CHECK-OBJ-NEXT:   - Index:   0
 # CHECK-OBJ-NEXT: Kind:FUNCTION
 # CHECK-OBJ-NEXT: Name:foo
-# CHECK-OBJ-NEXT: Flags:   [ EXPORTED ]
+# CHECK-OBJ-NEXT: Flags:   [ EXPORTED, NO_STRIP ]
 # CHECK-OBJ-NEXT: Function:0
Index: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
===
--- llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -722,6 +722,7 @@
   auto WasmSym = cast(Ctx.getOrCreateSymbol(SymName));
   WasmSym->setExportName(ExportName);
   TOut.emitExportName(WasmSym, ExportName);
+  Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip);
 }
 
 if (DirectiveID.getString() == ".import_module") {
Index: clang/test/CodeGen/wasm-export-name.c
===
--- clang/test/CodeGen/wasm-export-name.c
+++ clang/test/CodeGen/wasm-export-name.c
@@ -6,6 +6,8 @@
   return 43;
 }
 
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @foo to i8*)]
+
 // CHECK: define i32 @foo() [[A:#[0-9]+]]
 
 // CHECK: attributes [[A]] = {{{.*}} "wasm-export-name"="bar" {{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5772,8 +5772,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context)
-  WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(::new (S.Context) WebAssemblyExportNameAttr(S.Context, AL, Str));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 234142.
vsk added a comment.

Ignore an objc-cast report at a given SourceLocation after it's been reported 
once.


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

https://reviews.llvm.org/D71491

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/CodeGenObjC/for-in.m
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_value.cpp
  compiler-rt/lib/ubsan/ubsan_value.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m

Index: compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
@@ -0,0 +1,22 @@
+// REQUIRES: darwin
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast %s -O1 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast -fno-sanitize-recover=objc-cast %s -O1 -o %t.trap
+// RUN: not %run %t.trap 2>&1 | FileCheck %s
+
+#include 
+
+int main() {
+  NSArray *array = [NSArray arrayWithObjects: @1, @2, @3, (void *)0];
+  // CHECK: objc-cast.m:[[@LINE+1]]:{{.*}}: runtime error: invalid ObjC cast, object is a '__NSCFNumber', but expected a 'NSString'
+  for (NSString *str in array) {
+NSLog(@"%@", str);
+  }
+
+  // The diagnostic should only be printed once.
+  // CHECK-NOT: runtime error
+
+  return 0;
+}
Index: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
===
--- compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -109,6 +109,7 @@
 HANDLER(float_cast_overflow, "float-cast-overflow")
 HANDLER(load_invalid_value, "load-invalid-value")
 HANDLER(invalid_builtin, "invalid-builtin")
+HANDLER(invalid_objc_cast, "invalid-objc-cast")
 HANDLER(function_type_mismatch, "function-type-mismatch")
 HANDLER(implicit_conversion, "implicit-conversion")
 HANDLER(nonnull_arg, "nonnull-arg")
Index: compiler-rt/lib/ubsan/ubsan_value.h
===
--- compiler-rt/lib/ubsan/ubsan_value.h
+++ compiler-rt/lib/ubsan/ubsan_value.h
@@ -135,6 +135,9 @@
 /// \brief An opaque handle to a value.
 typedef uptr ValueHandle;
 
+/// Returns the class name of the given ObjC object, or null if the name
+/// cannot be found.
+const char *getObjCClassName(ValueHandle Pointer);
 
 /// \brief Representation of an operand value provided by the instrumented code.
 ///
Index: compiler-rt/lib/ubsan/ubsan_value.cpp
===
--- compiler-rt/lib/ubsan/ubsan_value.cpp
+++ compiler-rt/lib/ubsan/ubsan_value.cpp
@@ -16,9 +16,55 @@
 #include "ubsan_value.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_mutex.h"
+
+#if defined(__APPLE__)
+#include 
+#endif
 
 using namespace __ubsan;
 
+typedef const char *(*ObjCGetClassNameTy)(void *);
+
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want
+  // to introduce a static dependency from the ubsan runtime onto ObjC. Try to
+  // grab a handle to the ObjC runtime used by the process.
+  static bool AttemptedDlopen = false;
+  static void *ObjCHandle = nullptr;
+  static void *ObjCObjectGetClassName = nullptr;
+
+  // Prevent threads from racing to dlopen().
+  static __sanitizer::StaticSpinMutex Lock;
+  {
+__sanitizer::SpinMutexLock Guard();
+
+if (!AttemptedDlopen) {
+  ObjCHandle = dlopen(
+  "/usr/lib/libobjc.A.dylib",
+  RTLD_LAZY // Only bind symbols when used.
+  | RTLD_LOCAL  // Only make symbols available via the handle.
+  | RTLD_NOLOAD // Do not load the dylib, just grab a handle if the
+// image is already loaded.
+  | RTLD_FIRST  // Only search the image pointed-to by the handle.
+  );
+  AttemptedDlopen = true;
+  if (!ObjCHandle)
+return nullptr;
+  ObjCObjectGetClassName = dlsym(ObjCHandle, "object_getClassName");
+}
+  }
+
+  if (!ObjCObjectGetClassName)
+return nullptr;
+
+  return ObjCGetClassNameTy(ObjCObjectGetClassName)((void *)Pointer);
+#else
+  return nullptr;
+#endif
+}
+
 SIntMax Value::getSIntValue() const {
   CHECK(getType().isSignedIntegerTy());
   if (isInlineInt()) {
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- 

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 234139.
vsk added a comment.

Avoid a static initializer.


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

https://reviews.llvm.org/D71491

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/CodeGenObjC/for-in.m
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_value.cpp
  compiler-rt/lib/ubsan/ubsan_value.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m

Index: compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
@@ -0,0 +1,18 @@
+// REQUIRES: darwin
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast %s -O1 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast -fno-sanitize-recover=objc-cast %s -O1 -o %t.trap
+// RUN: not %run %t.trap 2>&1 | FileCheck %s
+
+#include 
+
+int main() {
+  NSArray *array = [NSArray arrayWithObjects: @1, @2, @3, (void *)0];
+  // CHECK: objc-cast.m:[[@LINE+1]]:{{.*}}: runtime error: invalid ObjC cast, object is a '__NSCFNumber', but expected a 'NSString'
+  for (NSString *str in array) {
+NSLog(@"%@", str);
+  }
+  return 0;
+}
Index: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
===
--- compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -109,6 +109,7 @@
 HANDLER(float_cast_overflow, "float-cast-overflow")
 HANDLER(load_invalid_value, "load-invalid-value")
 HANDLER(invalid_builtin, "invalid-builtin")
+HANDLER(invalid_objc_cast, "invalid-objc-cast")
 HANDLER(function_type_mismatch, "function-type-mismatch")
 HANDLER(implicit_conversion, "implicit-conversion")
 HANDLER(nonnull_arg, "nonnull-arg")
Index: compiler-rt/lib/ubsan/ubsan_value.h
===
--- compiler-rt/lib/ubsan/ubsan_value.h
+++ compiler-rt/lib/ubsan/ubsan_value.h
@@ -135,6 +135,9 @@
 /// \brief An opaque handle to a value.
 typedef uptr ValueHandle;
 
+/// Returns the class name of the given ObjC object, or null if the name
+/// cannot be found.
+const char *getObjCClassName(ValueHandle Pointer);
 
 /// \brief Representation of an operand value provided by the instrumented code.
 ///
Index: compiler-rt/lib/ubsan/ubsan_value.cpp
===
--- compiler-rt/lib/ubsan/ubsan_value.cpp
+++ compiler-rt/lib/ubsan/ubsan_value.cpp
@@ -16,9 +16,55 @@
 #include "ubsan_value.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_mutex.h"
+
+#if defined(__APPLE__)
+#include 
+#endif
 
 using namespace __ubsan;
 
+typedef const char *(*ObjCGetClassNameTy)(void *);
+
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want
+  // to introduce a static dependency from the ubsan runtime onto ObjC. Try to
+  // grab a handle to the ObjC runtime used by the process.
+  static bool AttemptedDlopen = false;
+  static void *ObjCHandle = nullptr;
+  static void *ObjCObjectGetClassName = nullptr;
+
+  // Prevent threads from racing to dlopen().
+  static __sanitizer::StaticSpinMutex Lock;
+  {
+__sanitizer::SpinMutexLock Guard();
+
+if (!AttemptedDlopen) {
+  ObjCHandle = dlopen(
+  "/usr/lib/libobjc.A.dylib",
+  RTLD_LAZY // Only bind symbols when used.
+  | RTLD_LOCAL  // Only make symbols available via the handle.
+  | RTLD_NOLOAD // Do not load the dylib, just grab a handle if the
+// image is already loaded.
+  | RTLD_FIRST  // Only search the image pointed-to by the handle.
+  );
+  AttemptedDlopen = true;
+  if (!ObjCHandle)
+return nullptr;
+  ObjCObjectGetClassName = dlsym(ObjCHandle, "object_getClassName");
+}
+  }
+
+  if (!ObjCObjectGetClassName)
+return nullptr;
+
+  return ObjCGetClassNameTy(ObjCObjectGetClassName)((void *)Pointer);
+#else
+  return nullptr;
+#endif
+}
+
 SIntMax Value::getSIntValue() const {
   CHECK(getType().isSignedIntegerTy());
   if (isInlineInt()) {
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -168,6 +168,14 @@
 /// Handle a builtin called in an invalid way.
 

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:953
+  SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+  SanitizerKind::LocalBounds | SanitizerKind::ObjCCast;
   if (getTriple().getArch() == llvm::Triple::x86 ||

delcypher wrote:
> `SanitizerKind::ObjCCast` doesn't seem to fit the comment above. It is 
> platform dependent (only really works for Apple platforms) and it **does** 
> require runtime support (i.e. the Objective-C runtime).
The runtime dependency is optional, and there's nothing inherently 
Apple-specific about this check. However, perhaps it's best not to 
inadvertently advertise support for the GNU objc runtime before it's in tree.



Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want

delcypher wrote:
> The compiler-rt codebase tends to use `SANITIZER_MAC` macro (defined to be 1 
> if Apple otherwise it's 0) rather than `__APPLE__`.
I see. That seems problematic, as it makes it tricky to add macOS-specific (or 
iOS-specific) functionality down the road. I don't think SANITIZER_MAC should 
be defined unless TARGET_OS_MACOS is true.



Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:37
+
+  if (!AttemptedDlopen) {
+ObjCHandle = dlopen(

delcypher wrote:
> You might want some sort of lock here (or atomic variable) to ensure we don't 
> race and try to `dlopen()` multiple times.
Yes, thanks.


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

https://reviews.llvm.org/D71491



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

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 234136.
vsk marked 3 inline comments as done.
vsk added a comment.

Address review feedback.


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

https://reviews.llvm.org/D71491

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/CodeGenObjC/for-in.m
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_value.cpp
  compiler-rt/lib/ubsan/ubsan_value.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m

Index: compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/objc-cast.m
@@ -0,0 +1,18 @@
+// REQUIRES: darwin
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast %s -O1 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+//
+// RUN: %clang -framework Foundation -fsanitize=objc-cast -fno-sanitize-recover=objc-cast %s -O1 -o %t.trap
+// RUN: not %run %t.trap 2>&1 | FileCheck %s
+
+#include 
+
+int main() {
+  NSArray *array = [NSArray arrayWithObjects: @1, @2, @3, (void *)0];
+  // CHECK: objc-cast.m:[[@LINE+1]]:{{.*}}: runtime error: invalid ObjC cast, object is a '__NSCFNumber', but expected a 'NSString'
+  for (NSString *str in array) {
+NSLog(@"%@", str);
+  }
+  return 0;
+}
Index: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
===
--- compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -109,6 +109,7 @@
 HANDLER(float_cast_overflow, "float-cast-overflow")
 HANDLER(load_invalid_value, "load-invalid-value")
 HANDLER(invalid_builtin, "invalid-builtin")
+HANDLER(invalid_objc_cast, "invalid-objc-cast")
 HANDLER(function_type_mismatch, "function-type-mismatch")
 HANDLER(implicit_conversion, "implicit-conversion")
 HANDLER(nonnull_arg, "nonnull-arg")
Index: compiler-rt/lib/ubsan/ubsan_value.h
===
--- compiler-rt/lib/ubsan/ubsan_value.h
+++ compiler-rt/lib/ubsan/ubsan_value.h
@@ -135,6 +135,9 @@
 /// \brief An opaque handle to a value.
 typedef uptr ValueHandle;
 
+/// Returns the class name of the given ObjC object, or null if the name
+/// cannot be found.
+const char *getObjCClassName(ValueHandle Pointer);
 
 /// \brief Representation of an operand value provided by the instrumented code.
 ///
Index: compiler-rt/lib/ubsan/ubsan_value.cpp
===
--- compiler-rt/lib/ubsan/ubsan_value.cpp
+++ compiler-rt/lib/ubsan/ubsan_value.cpp
@@ -16,9 +16,55 @@
 #include "ubsan_value.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_mutex.h"
+
+#if defined(__APPLE__)
+#include 
+#endif
 
 using namespace __ubsan;
 
+typedef const char *(*ObjCGetClassNameTy)(void *);
+
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want
+  // to introduce a static dependency from the ubsan runtime onto ObjC. Try to
+  // grab a handle to the ObjC runtime used by the process.
+  static bool AttemptedDlopen = false;
+  static void *ObjCHandle = nullptr;
+  static void *ObjCObjectGetClassName = nullptr;
+
+  // Prevent threads from racing to dlopen().
+  static __sanitizer::SpinMutex Lock;
+  {
+__sanitizer::GenericScopedLock<__sanitizer::SpinMutex> Guard();
+
+if (!AttemptedDlopen) {
+  ObjCHandle = dlopen(
+  "/usr/lib/libobjc.A.dylib",
+  RTLD_LAZY // Only bind symbols when used.
+  | RTLD_LOCAL  // Only make symbols available via the handle.
+  | RTLD_NOLOAD // Do not load the dylib, just grab a handle if the
+// image is already loaded.
+  | RTLD_FIRST  // Only search the image pointed-to by the handle.
+  );
+  AttemptedDlopen = true;
+  if (!ObjCHandle)
+return nullptr;
+  ObjCObjectGetClassName = dlsym(ObjCHandle, "object_getClassName");
+}
+  }
+
+  if (!ObjCObjectGetClassName)
+return nullptr;
+
+  return ObjCGetClassNameTy(ObjCObjectGetClassName)((void *)Pointer);
+#else
+  return nullptr;
+#endif
+}
+
 SIntMax Value::getSIntValue() const {
   CHECK(getType().isSignedIntegerTy());
   if (isInlineInt()) {
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -168,6 +168,14 

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71020



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

[PATCH] D71566: New checks for fortified sprintf

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: erik.pilkington.
serge-sans-paille added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

Implement a pessimistic evaluator of the minimal required size for a buffer 
based on the format string, and couple that with the fortified version to emit 
a warning when the buffer size is lower than the lower bound computed from the 
format string.

See the test cases for examples of warnings, and 
https://github.com/serge-sans-paille/llvm-project/pull/6/checks for the 
cross-platform validation.

Note: The lower bound could be improved, but I'd rather do that in an 
incremental commit, if that's okay with the reviewers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71566

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -96,6 +96,21 @@
   __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
 }
 
+void call_sprintf_chk(char *buf) {
+  __builtin___sprintf_chk(buf, 1, 2, "a%d", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "a%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 4, "a%#x", 9);
+  __builtin___sprintf_chk(buf, 1, 3, "a%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 4, "a%p", (void *)9);
+  __builtin___sprintf_chk(buf, 1, 3, "a%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 3, "a%+d", 9);
+  __builtin___sprintf_chk(buf, 1, 2, "a%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 3}}
+  __builtin___sprintf_chk(buf, 1, 6, "a%5d", 9);
+  __builtin___sprintf_chk(buf, 1, 5, "a%5d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 2, "%f", 9.f);
+  __builtin___sprintf_chk(buf, 1, 1, "a%f", 9.f); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+}
+
 #ifdef __cplusplus
 template  struct S {
   void mf() const {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -308,6 +308,58 @@
 
   return false;
 }
+class EstimateSizeFormatHandler
+: public analyze_format_string::FormatStringHandler {
+  size_t Size;
+
+public:
+  EstimateSizeFormatHandler(const StringLiteral *fexpr)
+  : Size(fexpr->getLength()) {}
+
+  bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier ,
+ const char *startSpecifier,
+ unsigned specifierLen) override {
+const analyze_format_string::OptionalAmount  = FS.getFieldWidth();
+if (FW.getHowSpecified() == analyze_format_string::OptionalAmount::Constant)
+  Size += FW.getConstantAmount();
+else
+  Size += FS.getConversionSpecifier().isAnyIntArg() ||
+  FS.getConversionSpecifier().isDoubleArg();
+Size += FS.hasPlusPrefix() || FS.hasSpacePrefix();
+if (FS.hasAlternativeForm()) {
+  switch (FS.getConversionSpecifier().getKind()) {
+  default:
+break;
+  case analyze_format_string::ConversionSpecifier::oArg: // leading 0
+  case analyze_format_string::ConversionSpecifier::aArg: // force a .
+  case analyze_format_string::ConversionSpecifier::AArg: // force a .
+  case analyze_format_string::ConversionSpecifier::eArg: // force a .
+  case analyze_format_string::ConversionSpecifier::EArg: // force a .
+  case analyze_format_string::ConversionSpecifier::fArg: // force a .
+  case analyze_format_string::ConversionSpecifier::FArg: // force a .
+  case analyze_format_string::ConversionSpecifier::gArg: // force a .
+  case analyze_format_string::ConversionSpecifier::GArg: // force a .
+Size += 1;
+break;
+  case analyze_format_string::ConversionSpecifier::xArg: // leading 0x
+  case analyze_format_string::ConversionSpecifier::XArg: // leading 0x
+Size += 2;
+break;
+  }
+}
+switch (FS.getConversionSpecifier().getKind()) {
+case analyze_format_string::ConversionSpecifier::pArg: // leading 0x
+  Size += 3;
+  break;
+ 

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Most probably, we can use this solution without adding a new expression. 
`DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being used. You 
can use FoundDecl to point to the original function and used decl to point to 
the function being called in this context. But at first, we need to be sure 
that we can handle all corner cases correctly.

For example, can it handle such cases as:
t1.c:

  int hst();
  #pragma omp declare variant(hst) match(device={kind(host)})
  int base();

t2.c:

  int main() {
return base();
  }

This is the correct C code, I assume. At least, clang compiles it.

Another problem:
t1.c:

  int hst();
  #pragma omp declare varint(hst) match(device={kind(host)})
  int base();

t2.c:

  int base() { return 0; }
  int main() {
return base();
  }

According to the standard, this is valid code and `hst` function must be called 
(though it is not correct since in C/C++ each definition is a declaration and 
all restriction applied to the declaration must be applied to the definition 
too).

Another one possible problem might be with the templates:

  template  T base() { return T(); }
  int hst() { return 1; }
  
  int main() {
return base();
  }
  
  #pragma omp declare variant(hst) match(device={kind(gpu)})
  template
  T base();
  
  int foo() {
return base();
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[clang] 803403a - Fix a bug in the property-based serialization of

[John McCall via cfe-commits]

Author: John McCall
Date: 2019-12-16T16:08:09-05:00
New Revision: 803403afc83f659be1c620eb1896dcf704b18b0a

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

LOG: Fix a bug in the property-based serialization of
dependent template names.

Apparently we didn't test this in the test suite because we have
a lot of redundant ways of representing this situation that kick
in in the more common situations.  For example, DependentTST stores
a qualifier + identifier pair rather than a TemplateName.

Added: 


Modified: 
clang/include/clang/AST/PropertiesBase.td
clang/test/PCH/cxx-templates.cpp
clang/test/PCH/cxx-templates.h

Removed: 




diff  --git a/clang/include/clang/AST/PropertiesBase.td 
b/clang/include/clang/AST/PropertiesBase.td
index 1cd1c79d2358..9aacdb9fee36 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -362,7 +362,7 @@ let Class = PropertyTypeCase in {
   : nullptr) }];
   }
   def : Property<"operatorKind", OverloadedOperatorKind> {
-let Conditional = [{ identifier }];
+let Conditional = [{ !identifier }];
 let Read = [{ dtn->getOperator() }];
   }
   def : Creator<[{

diff  --git a/clang/test/PCH/cxx-templates.cpp 
b/clang/test/PCH/cxx-templates.cpp
index 966bd00b858c..cd2787d90359 100644
--- a/clang/test/PCH/cxx-templates.cpp
+++ b/clang/test/PCH/cxx-templates.cpp
@@ -165,3 +165,13 @@ namespace DependentMemberExpr {
   static_assert(A().f() == 1); // expected-error {{static_assert failed}}
 #endif
 }
+
+namespace DependentTemplateName {
+  struct HasMember {
+template  struct Member;
+  };
+
+  void test() {
+getWithIdentifier();
+  }
+}

diff  --git a/clang/test/PCH/cxx-templates.h b/clang/test/PCH/cxx-templates.h
index 5aa68546a429..b4ea2c23b3cc 100644
--- a/clang/test/PCH/cxx-templates.h
+++ b/clang/test/PCH/cxx-templates.h
@@ -448,3 +448,11 @@ namespace DependentMemberExpr {
 constexpr int f() { return Base::setstate(); }
   };
 }
+
+namespace DependentTemplateName {
+  template  class Template>
+  struct TakesClassTemplate {};
+
+  template 
+  TakesClassTemplate getWithIdentifier();
+}



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

Re: [clang] 00bc76e - Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.

[John McCall via cfe-commits]

On 16 Dec 2019, at 15:11, John McCall wrote:

On 16 Dec 2019, at 15:07, Vedant Kumar wrote:

Hi John,

The lldb bot went red after your clang AST changes landed: 
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4801/changes#detail0 



All of the test failures were due to a clang assertion:

Assertion failed: (isOverloadedOperator() && "Template name isn't an 
overloaded operator?"), function getOperator, file 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/include/clang/AST/TemplateName.h, 
line 520.


Perhaps this is related to "Commit a9db0d9f - Use property-based 
serialization for TemplateName"?


Sorry, I’ll take a look.


Should be fixed in 803403afc83f659be1c620eb1896dcf704b18b0a.

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

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

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: clang/include/clang/AST/Type.h:477-479
+   ((isPtrSizeAddressSpace(A) && B == LangAS::Default) ||
+(isPtrSizeAddressSpace(B) && A == LangAS::Default) ||
+(isPtrSizeAddressSpace(A) && isPtrSizeAddressSpace(B)));

rnk wrote:
> Can this be simplified to:
>   ((isPtrSizeAddressSpace(A) || A == LangAS::Default) &&
>(isPtrSizeAddressSpace(B) || B == LangAS::Default))
> Mainly I wanted to avoid recomputing isPtrSizeAddressSpace for A and B.
> 
> I think it's only not equivalent when A and B are both default, but we 
> already return true in that case.
Yes -- I think I considered doing this and then forgot that we already return 
true when A and B are both default. 



Comment at: clang/lib/Sema/SemaDecl.cpp:3156
+
+static bool HasSameFunctionTypeIgnoringPointerSizes(ASTContext ,
+QualType Old,

rnk wrote:
> I wonder if the simplest way to express this would be to follow the pattern 
> of getFunctionTypeWithExceptionSpec and 
> hasSameFunctionTypeIgnoringExceptionSpec, i.e. make a function that strips 
> pointer sized address spaces off of pointer typed arguments, returns it, and 
> then compare them. ASTContext would be a natural place for that kind of type 
> adjustment.
Done, this does make the code a bit shorter. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71039



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

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

My primary concern with this is that I'm not sure you should be passing this 
information separately, as opposed to encoding it into the bitcode.

On ARM, the ABI for a file is generally derived from the target triple.  For 
example, "arm-eabi" is soft-float, "arm-eabihf" is hard-float.  This makes the 
intended target clear, provides some protection against accidentally using LTO 
with files compiled for different ABIs, and matches up well with our existing 
configuration flags to set a default target for a compiler.

For other ABI-ish sorts of questions, LLVM IR also supports module flags, which 
can specify various parameters that apply to an entire object file.  See, for 
example, http://llvm.org/docs/LangRef.html#c-type-width-module-flags-metadata . 
 This has some of the same benefits: it clearly applies to the whole file, and 
it detects mixing targets with LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387



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

[clang] b1e542f - [NFC-I] Remove hack for fp-classification builtins

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2019-12-16T12:22:55-08:00
New Revision: b1e542f302c1ed796ad9f703d4d36e010afcb914

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

LOG: [NFC-I] Remove hack for fp-classification builtins

The FP-classification builtins (__builtin_isfinite, etc) use variadic
packs in the definition file to mean an overload set.  Because of that,
floats were converted to doubles, which is incorrect. There WAS a patch
to remove the cast after the fact.

THis patch switches these builtins to just be custom type checking,
calls the implicit conversions for the integer members, and makes sure
the correct L->R casts are put into place, then does type checking like
normal.

A future direction (that wouldn't be NFC) would consider making
conversions for the floating point parameter legal.

Added: 
clang/test/Sema/builtin-fpclassification.c

Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/crash-invalid-builtin.c

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 29dec78e4b96..51d3500df8ae 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -399,15 +399,15 @@ BUILTIN(__builtin_islessgreater , "i.", "Fnct")
 BUILTIN(__builtin_isunordered   , "i.", "Fnct")
 
 // Unary FP classification
-BUILTIN(__builtin_fpclassify, "ii.", "Fnc")
-BUILTIN(__builtin_isfinite,   "i.", "Fnc")
-BUILTIN(__builtin_isinf,  "i.", "Fnc")
-BUILTIN(__builtin_isinf_sign, "i.", "Fnc")
-BUILTIN(__builtin_isnan,  "i.", "Fnc")
-BUILTIN(__builtin_isnormal,   "i.", "Fnc")
+BUILTIN(__builtin_fpclassify, "ii.", "Fnct")
+BUILTIN(__builtin_isfinite,   "i.", "Fnct")
+BUILTIN(__builtin_isinf,  "i.", "Fnct")
+BUILTIN(__builtin_isinf_sign, "i.", "Fnct")
+BUILTIN(__builtin_isnan,  "i.", "Fnct")
+BUILTIN(__builtin_isnormal,   "i.", "Fnct")
 
 // FP signbit builtins
-BUILTIN(__builtin_signbit, "i.", "Fnc")
+BUILTIN(__builtin_signbit, "i.", "Fnct")
 BUILTIN(__builtin_signbitf, "if", "Fnc")
 BUILTIN(__builtin_signbitl, "iLd", "Fnc")
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index aff63aef2934..80d8a486e4cf 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5796,51 +5796,35 @@ bool Sema::SemaBuiltinFPClassification(CallExpr 
*TheCall, unsigned NumArgs) {
<< SourceRange(TheCall->getArg(NumArgs)->getBeginLoc(),
   (*(TheCall->arg_end() - 1))->getEndLoc());
 
+  // __builtin_fpclassify is the only case where NumArgs != 1, so we can count
+  // on all preceding parameters just being int.  Try all of those.
+  for (unsigned i = 0; i < NumArgs - 1; ++i) {
+Expr *Arg = TheCall->getArg(i);
+
+if (Arg->isTypeDependent())
+  return false;
+
+ExprResult Res = PerformImplicitConversion(Arg, Context.IntTy, AA_Passing);
+
+if (Res.isInvalid())
+  return true;
+TheCall->setArg(i, Res.get());
+  }
+
   Expr *OrigArg = TheCall->getArg(NumArgs-1);
 
   if (OrigArg->isTypeDependent())
 return false;
 
+  OrigArg = DefaultFunctionArrayLvalueConversion(OrigArg).get();
+  TheCall->setArg(NumArgs - 1, OrigArg);
+
   // This operation requires a non-_Complex floating-point number.
   if (!OrigArg->getType()->isRealFloatingType())
 return Diag(OrigArg->getBeginLoc(),
 diag::err_typecheck_call_invalid_unary_fp)
<< OrigArg->getType() << OrigArg->getSourceRange();
 
-  // If this is an implicit conversion from float -> float, double, or
-  // long double, or half -> half, float, double, or long double, remove it.
-  if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
-// Only remove standard FloatCasts, leaving other casts inplace
-if (Cast->getCastKind() == CK_FloatingCast) {
-  bool IgnoreCast = false;
-  Expr *CastArg = Cast->getSubExpr();
-  if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
-assert(
-(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
-"promotion from float to either float, double, or long double is "
-"the only expected cast here");
-IgnoreCast = true;
-  } else if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Half) 
&&
- !Context.getTargetInfo().useFP16ConversionIntrinsics()) {
-assert(
-(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
- 

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

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 234125.
akhuang marked 8 inline comments as done.
akhuang added a comment.

- Added docs for __ptr32, __ptr64, __sptr, __utr
- Moved some functions into ASTContext
- and addressed other comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71039

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AddressSpaces.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/ms-mixed-ptr-sizes.c
  clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/SemaTemplate/address_space-dependent.cpp

Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@
 
 template 
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388598)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388595)}}
 }
 
 template 
@@ -101,7 +101,7 @@
   car<1, 2, 3>(); // expected-note {{in instantiation of function template specialization 'car<1, 2, 3>' requested here}}
   HasASTemplateFields<1> HASTF;
   neg<-1>(); // expected-note {{in instantiation of function template specialization 'neg<-1>' requested here}}
-  correct<0x76>();
+  correct<0x73>();
   tooBig<8388650>(); // expected-note {{in instantiation of function template specialization 'tooBig<8388650>' requested here}}
 
   __attribute__((address_space(1))) char *x;
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -150,6 +150,20 @@
 void ptr_func2(int * __sptr __ptr32 i) {}  // expected-note {{previous definition is here}}
 void ptr_func2(int * __uptr __ptr32 i) {} // expected-error {{redefinition of 'ptr_func2'}}
 
+// Check for warning when return types have the type attribute.
+void *__ptr32 ptr_func3() { return 0; } // expected-note {{previous definition is here}}
+void *__ptr64 ptr_func3() { return 0; } // expected-error {{redefinition of 'ptr_func3'}}
+
+// Test that __ptr32/__ptr64 can be passed as arguments with other address
+// spaces.
+void ptr_func4(int *i);
+void ptr_func5(int *__ptr32 i);
+void test_ptr_arguments() {
+  int *__ptr64 i64;
+  ptr_func4(i64);
+  ptr_func5(i64);
+}
+
 int * __sptr __ptr32 __sptr wrong4; // expected-warning {{attribute '__sptr' is already applied}}
 
 __ptr32 int *wrong5; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
Index: clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fms-extensions -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -fms-extensions -emit-llvm -triple x86_64-windows-msvc -o - %s | FileCheck %s --check-prefixes=WIN
+
+// CHECK-LABEL: define {{.*}}void @_Z2f0PU10ptr32_sptri
+// WIN-LABEL: define {{.*}}void @"?f0@@YAXPAH@Z"
+void f0(int * __ptr32 p) {}
+
+// CHECK-LABEL: define {{.*}}i8 addrspace(271)* @_Z2f1PU10ptr32_sptri
+// WIN-LABEL: define {{.*}}i8 addrspace(271)* @"?f1@@YAPAXPAH@Z"
+void * __ptr32 __uptr f1(int * __ptr32 p) { return 0; }
+
+// CHECK-LABEL: define {{.*}}void @_Z2f2Pi
+// WIN-LABEL: define {{.*}}void @"?f2@@YAXPEAH@Z"
+void f2(int * __ptr64 p) {}
+
+  // CHECK-LABEL: define {{.*}}i8* @_Z2f3Pi
+// WIN-LABEL: define {{.*}}i8* @"?f3@@YAPEAXPEAH@Z"
+void * __ptr64 f3(int * __ptr64 p) { return 0; }
Index: clang/test/CodeGen/ms-mixed-ptr-sizes.c
===
--- /dev/null
+++ clang/test/CodeGen/ms-mixed-ptr-sizes.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -emit-llvm -O2 \
+// RUN:   < %s | FileCheck %s --check-prefixes=X64,CHECK
+// RUN: %clang_cc1 -triple i386-pc-win32 -fms-extensions -emit-llvm -O2 \
+// RUN:   < %s | FileCheck %s --check-prefixes=X86,CHECK
+
+struct Foo {
+  int * __ptr32 p32;
+  int * __ptr64 p64;
+};
+void use_foo(struct Foo *f);
+void test_sign_ext(struct Foo 

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Do you have any particular users/use case for this?

Might be best as two separate patches (one for the LLVM change, one for the 
Clang change) with tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71451



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy )
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, ,
-  ),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  , ),

lildmh wrote:
> ABataev wrote:
> > Do we really need to set `HasMapper` to `true` unconditionally here and in 
> > other places?
> For `map`, `to`, and `from` clauses, they can have mappers, so it's set to 
> `true`. For `is_device_ptr` and `use_device_ptr`, it's set to `false`.
So, it completely depends on the clause kind, right? If so, can we just remove 
it and rely on the clause kind?


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

https://reviews.llvm.org/D67833



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

[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not sure it's legal to transform a non-faulting load to a load with a 
non-faulting flag?  At least, we'd need to consider the implications of that 
very carefully.  In particular, I'm concerned about the interaction with 
intrinsics that read/write FFR.  I mean, you could specify that loads marked 
MONonFaulting actually write to the FFR register, but that seems confusing.

It seems simpler to preserve the intrinsic until isel, at least for now.




Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4490
+return getConstant(Val.zextOrTrunc(VT.getScalarSizeInBits()), DL, VT,
+   C->isTargetOpcode(), C->isOpaque());
+  case ISD::SIGN_EXTEND:

I'm not sure how this is related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71556



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

lildmh marked 5 inline comments as done.
lildmh added a comment.

Alexey, thanks for the review




Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy )
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, ,
-  ),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  , ),

ABataev wrote:
> Do we really need to set `HasMapper` to `true` unconditionally here and in 
> other places?
For `map`, `to`, and `from` clauses, they can have mappers, so it's set to 
`true`. For `is_device_ptr` and `use_device_ptr`, it's set to `false`.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

ABataev wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > CamelCase
> > > > Seems to me, with mapper you're changing the contract of this function. 
> > > > It is supposed to return the size in bytes, with Mapper it returns just 
> > > > number of elements. Bad decision. Better to convert size in bytes to 
> > > > number of elements in place where you need it.
> > > Yes. The reason why I did this is I found this is the single place that I 
> > > can consider all situations. Otherwise, I need to divide the size by the 
> > > element size in other places. Since this function has discussed all 
> > > situations for the element size, I will need to duplicate the work 
> > > elsewhere if I don't do it here. The latter solution is not good for code 
> > > mountainous I think.
> > Changing the original function contract is also not the best solution. 
> > Better to introduce the new function. If some of the code can be reused, 
> > need to factor out the common code and reuse it in the old function and the 
> > new one.
> Also, additional question. Does it mean that we pass to the runtime functions 
> different values in different situations: in one case we pass size in bytes, 
> with mappers - number of elements? If so, it is a very bad idea caused by bad 
> design. We need to pass either size in bytes, or number of elements in this 
> argument.
Yes, the current design is like this. Let me see how to make it better



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8821-8822
+
+// No user-defined mapper for default mapping.
+CurMappers.push_back(nullptr);
   }

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > In general, there should not be branch for default mapping during the 
> > > codegen, the implicit map clauses must be generated in sema.
> > Not sure where this code will be used. I guess it's still correct to add a 
> > null to the mappers here?
> I mean, that in general, it would be good to see here some kind of an 
> assertion. Almost all situations must be resolved in Sema.
Okay, I guess it's not the work of this patch


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

https://reviews.llvm.org/D67833



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70524#1783363 , @probinson wrote:

> > Hmm, maybe this feature/suggestion is broken or at least not exactly 
> > awesome when it comes to auto-returning functions that are eventually 
> > void-returning functions? Now the function definition has no DW_AT_type to 
> > override the unspecified_type in the declaration... :/ that's unfortunate 
> > (@probinson - thoughts?)
>
> Normally, the DW_AT_specification on the definition would mean, look at the 
> declaration for additional attributes, such as DW_AT_type.  However, the 
> declaration's unspecified_type means, look at the definition.  The definition 
> omits DW_AT_type, therefore the return type is "void".
>  It's a wee bit circular, but I think it's not unreasonable to expect the 
> consumer to figure this out.


Fair enough. I think that's pretty awkward, but don't feel strongly enough to 
say that's bad/wrong - it's what GCC does, it's probably what GDB understands, 
etc.

Test cases/functionality need to be fixed here, because currently the deduced 
type isn't being carried on the function definition (it just "works out" for 
the limited testing provided - because the test is testing void return, but 
adding/changing the test to test non-void return should expose the bug)

Also - I think this probably should be enabled even pre-DWARFv5. GDB does that 
and unspecified_type is available pre-v5 & it saves having a divergence in the 
frontend for v5 generation which I think is probably good (I don't think we 
have that as a hard line, but I'd tend to think it's a direction we should 
generally lean in - where the DWARF version is handled by the metadata 
attribute and the backend for the most part/where possible).


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

https://reviews.llvm.org/D70524



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy )
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, ,
-  ),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  , ),

Do we really need to set `HasMapper` to `true` unconditionally here and in 
other places?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:841-842
+  /// Get the function for the specified user-defined mapper, if any.
+  virtual llvm::Function *
+  getUserDefinedMapperFunc(const OMPDeclareMapperDecl *D);
 

Is this required to make it `virtual`?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:1556
   return BasePointersArray && PointersArray && SizesArray &&
- MapTypesArray && NumberOfPtrs;
+ MapTypesArray && MappersArray && NumberOfPtrs;
 }

`(!HasMapper || MappersArray)`?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

lildmh wrote:
> ABataev wrote:
> > ABataev wrote:
> > > CamelCase
> > Seems to me, with mapper you're changing the contract of this function. It 
> > is supposed to return the size in bytes, with Mapper it returns just number 
> > of elements. Bad decision. Better to convert size in bytes to number of 
> > elements in place where you need it.
> Yes. The reason why I did this is I found this is the single place that I can 
> consider all situations. Otherwise, I need to divide the size by the element 
> size in other places. Since this function has discussed all situations for 
> the element size, I will need to duplicate the work elsewhere if I don't do 
> it here. The latter solution is not good for code mountainous I think.
Changing the original function contract is also not the best solution. Better 
to introduce the new function. If some of the code can be reused, need to 
factor out the common code and reuse it in the old function and the new one.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > CamelCase
> > > Seems to me, with mapper you're changing the contract of this function. 
> > > It is supposed to return the size in bytes, with Mapper it returns just 
> > > number of elements. Bad decision. Better to convert size in bytes to 
> > > number of elements in place where you need it.
> > Yes. The reason why I did this is I found this is the single place that I 
> > can consider all situations. Otherwise, I need to divide the size by the 
> > element size in other places. Since this function has discussed all 
> > situations for the element size, I will need to duplicate the work 
> > elsewhere if I don't do it here. The latter solution is not good for code 
> > mountainous I think.
> Changing the original function contract is also not the best solution. Better 
> to introduce the new function. If some of the code can be reused, need to 
> factor out the common code and reuse it in the old function and the new one.
Also, additional question. Does it mean that we pass to the runtime functions 
different values in different situations: in one case we pass size in bytes, 
with mappers - number of elements? If so, it is a very bad idea caused by bad 
design. We need to pass either size in bytes, or number of elements in this 
argument.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7564-7573
   void generateInfoForComponentList(
-  OpenMPMapClauseKind MapType,
-  ArrayRef MapModifiers,
+  OpenMPMapClauseKind MapType, ArrayRef 
MapModifiers,
   OMPClauseMappableExprCommon::MappableExprComponentListRef Components,
   MapBaseValuesArrayTy , MapValuesArrayTy ,
   MapValuesArrayTy , MapFlagsArrayTy ,
-  StructRangeInfoTy , bool IsFirstComponentList,
-  bool IsImplicit,
+  MapMappersArrayTy , StructRangeInfoTy ,
+  bool IsFirstComponentList, bool IsImplicit,

lildmh wrote:
> ABataev wrote:
> > Too many params in the function, worth thinking about wrapping them in a 
> > record.
> Maybe not in this patch?
It is better to make it in this patch or make a separate patch and commit it 
before this one. We try to resolve this as soon as possible.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8821-8822
+
+// No user-defined mapper for default mapping.
+CurMappers.push_back(nullptr);
   }

Re: [clang] 00bc76e - Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.

[John McCall via cfe-commits]

On 16 Dec 2019, at 15:07, Vedant Kumar wrote:

Hi John,

The lldb bot went red after your clang AST changes landed: 
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4801/changes#detail0 



All of the test failures were due to a clang assertion:

Assertion failed: (isOverloadedOperator() && "Template name isn't an 
overloaded operator?"), function getOperator, file 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/include/clang/AST/TemplateName.h, 
line 520.


Perhaps this is related to "Commit a9db0d9f - Use property-based 
serialization for TemplateName"?


Sorry, I’ll take a look.

John.



vedant

p.s: I would have responded to that commit email directly, but for 
some reason it isn't in my inbox. Tom, do you know if there have been 
any issues with missing emails lately?


On Dec 16, 2019, at 10:34 AM, John McCall via cfe-commits 
 wrote:



Author: John McCall
Date: 2019-12-16T13:33:59-05:00
New Revision: 00bc76edddb5a6cd417610e96289a5dc15245867

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


LOG: Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.

I'm going to introduce some uses of the property read/write methods.

Added:


Modified:
   clang/utils/TableGen/ClangASTPropertiesEmitter.cpp

Removed:




diff  --git a/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp 
b/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp

index c80b10e538cd..e14b9eb6e6ff 100644
--- a/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
+++ b/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
@@ -84,6 +84,7 @@ class ASTPropsEmitter {
raw_ostream 
RecordKeeper 
std::map NodeInfos;
+  std::vector AllPropertyTypes;

public:
ASTPropsEmitter(RecordKeeper , raw_ostream )
@@ -124,6 +125,15 @@ class ASTPropsEmitter {
  info.Override = overrideRule;
}

+for (PropertyType type :
+   records.getAllDerivedDefinitions(PropertyTypeClassName)) 
{
+  // Ignore generic specializations; they're generally not 
useful when

+  // emitting basic emitters etc.
+  if (type.isGenericSpecialization()) continue;
+
+  AllPropertyTypes.push_back(type);
+}
+
Validator(*this).validate();
}

@@ -175,6 +185,11 @@ class ASTPropsEmitter {
  void emitReadOfProperty(Property property);
  void emitWriteOfProperty(Property property);

+  void emitBasicReaderWriterFile(const ReaderWriterInfo );
+  void emitDispatcherTemplate(const ReaderWriterInfo );
+  void emitPackUnpackOptionalTemplate(const ReaderWriterInfo );
+  void emitBasicReaderWriterTemplate(const ReaderWriterInfo );
+
private:
  class Validator {
const ASTPropsEmitter 
@@ -477,11 +492,11 @@ void clang::EmitClangTypeWriter(RecordKeeper 
, raw_ostream ) {
/*** BASIC READER/WRITERS 
***/

//

-static void emitDispatcherTemplate(ArrayRef types, 
raw_ostream ,

-   const ReaderWriterInfo ) {
+void
+ASTPropsEmitter::emitDispatcherTemplate(const ReaderWriterInfo 
) {

  // Declare the {Read,Write}Dispatcher template.
  StringRef dispatcherPrefix = (info.IsReader ? "Read" : "Write");
-  out << "template \n"
+  Out << "template \n"
 "struct " << dispatcherPrefix << "Dispatcher;\n";

  // Declare a specific specialization of the dispatcher template.
@@ -490,10 +505,10 @@ static void 
emitDispatcherTemplate(ArrayRef types, raw_ostream ,

const Twine ,
StringRef methodSuffix) {
StringRef var = info.HelperVariable;
-out << "template " << specializationParameters << "\n"
+Out << "template " << specializationParameters << "\n"
   "struct " << dispatcherPrefix << "Dispatcher<"
 << cxxTypeName << "> {\n";
-out << "  template class... Args>\n"
+Out << "  template class... Args>\n"
   "  static " << (info.IsReader ? cxxTypeName : "void") << " 
"

   << info.MethodPrefix
   << "(Basic" << info.ClassSuffix << " &" << var
@@ -506,7 +521,7 @@ static void 
emitDispatcherTemplate(ArrayRef types, raw_ostream ,

  };

  // Declare explicit specializations for each of the concrete types.
-  for (PropertyType type : types) {
+  for (PropertyType type : AllPropertyTypes) {
declareSpecialization("<>",
  type.getCXXTypeName(),
  type.getAbstractTypeName());
@@ -524,22 +539,21 @@ static void 
emitDispatcherTemplate(ArrayRef types, raw_ostream ,

  declareSpecialization("",
"llvm::Optional",
"Optional");
-  out << "\n";
+  Out << "\n";
}

[PATCH] D71500: [WebAssembly] Replace SIMD int min/max builtins with patterns

[Thomas Lively via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a93756dfbb0: [WebAssembly] Replace SIMD int min/max 
builtins with patterns (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71500

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-arith.ll
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll

Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -65,46 +65,6 @@
   ret <16 x i8> %a
 }
 
-; CHECK-LABEL: min_s_v16i8:
-; SIMD128-NEXT: .functype min_s_v16i8 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i8x16.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8>, <16 x i8>)
-define <16 x i8> @min_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
-  %a = call <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
-  ret <16 x i8> %a
-}
-
-; CHECK-LABEL: min_u_v16i8:
-; SIMD128-NEXT: .functype min_u_v16i8 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i8x16.min_u $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8>, <16 x i8>)
-define <16 x i8> @min_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
-  %a = call <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
-  ret <16 x i8> %a
-}
-
-; CHECK-LABEL: max_s_v16i8:
-; SIMD128-NEXT: .functype max_s_v16i8 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i8x16.max_s $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8>, <16 x i8>)
-define <16 x i8> @max_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
-  %a = call <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
-  ret <16 x i8> %a
-}
-
-; CHECK-LABEL: max_u_v16i8:
-; SIMD128-NEXT: .functype max_u_v16i8 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i8x16.max_u $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8>, <16 x i8>)
-define <16 x i8> @max_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
-  %a = call <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
-  ret <16 x i8> %a
-}
-
 ; CHECK-LABEL: any_v16i8:
 ; SIMD128-NEXT: .functype any_v16i8 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i8x16.any_true $push[[R:[0-9]+]]=, $0{{$}}
@@ -208,46 +168,6 @@
   ret <8 x i16> %a
 }
 
-; CHECK-LABEL: min_s_v8i16:
-; SIMD128-NEXT: .functype min_s_v8i16 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i16x8.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.min.signed.v8i16(<8 x i16>, <8 x i16>)
-define <8 x i16> @min_s_v8i16(<8 x i16> %x, <8 x i16> %y) {
-  %a = call <8 x i16> @llvm.wasm.min.signed.v8i16(<8 x i16> %x, <8 x i16> %y)
-  ret <8 x i16> %a
-}
-
-; CHECK-LABEL: min_u_v8i16:
-; SIMD128-NEXT: .functype min_u_v8i16 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i16x8.min_u $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.min.unsigned.v8i16(<8 x i16>, <8 x i16>)
-define <8 x i16> @min_u_v8i16(<8 x i16> %x, <8 x i16> %y) {
-  %a = call <8 x i16> @llvm.wasm.min.unsigned.v8i16(<8 x i16> %x, <8 x i16> %y)
-  ret <8 x i16> %a
-}
-
-; CHECK-LABEL: max_s_v8i16:
-; SIMD128-NEXT: .functype max_s_v8i16 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i16x8.max_s $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.max.signed.v8i16(<8 x i16>, <8 x i16>)
-define <8 x i16> @max_s_v8i16(<8 x i16> %x, <8 x i16> %y) {
-  %a = call <8 x i16> @llvm.wasm.max.signed.v8i16(<8 x i16> %x, <8 x i16> %y)
-  ret <8 x i16> %a
-}
-
-; CHECK-LABEL: max_u_v8i16:
-; SIMD128-NEXT: .functype max_u_v8i16 (v128, v128) -> (v128){{$}}
-; SIMD128-NEXT: i16x8.max_u $push[[R:[0-9]+]]=, $0, $1{{$}}
-; SIMD128-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.max.unsigned.v8i16(<8 x i16>, <8 x i16>)
-define <8 x i16> @max_u_v8i16(<8 x i16> %x, <8 x i16> %y) {
-  %a = call <8 x i16> @llvm.wasm.max.unsigned.v8i16(<8 x i16> %x, <8 x i16> %y)
-  ret <8 x i16> %a
-}
-
 ; CHECK-LABEL: any_v8i16:
 ; SIMD128-NEXT: .functype any_v8i16 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i16x8.any_true $push[[R:[0-9]+]]=, $0{{$}}
@@ -347,46 +267,6 @@
 ; ==
 ; 4 x i32
 ; ==
-; CHECK-LABEL: 

Re: [clang] 00bc76e - Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.

[Vedant Kumar via cfe-commits]

Hi John,

The lldb bot went red after your clang AST changes landed: 
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4801/changes#detail0 


All of the test failures were due to a clang assertion:

Assertion failed: (isOverloadedOperator() && "Template name isn't an overloaded 
operator?"), function getOperator, file 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/include/clang/AST/TemplateName.h,
 line 520.

Perhaps this is related to "Commit a9db0d9f - Use property-based serialization 
for TemplateName"?

vedant

p.s: I would have responded to that commit email directly, but for some reason 
it isn't in my inbox. Tom, do you know if there have been any issues with 
missing emails lately?

> On Dec 16, 2019, at 10:34 AM, John McCall via cfe-commits 
>  wrote:
> 
> 
> Author: John McCall
> Date: 2019-12-16T13:33:59-05:00
> New Revision: 00bc76edddb5a6cd417610e96289a5dc15245867
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/00bc76edddb5a6cd417610e96289a5dc15245867
> DIFF: 
> https://github.com/llvm/llvm-project/commit/00bc76edddb5a6cd417610e96289a5dc15245867.diff
> 
> LOG: Move Basic{Reader,Writer} emission into ASTPropsEmitter; NFC.
> 
> I'm going to introduce some uses of the property read/write methods.
> 
> Added: 
> 
> 
> Modified: 
>clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp 
> b/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
> index c80b10e538cd..e14b9eb6e6ff 100644
> --- a/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
> +++ b/clang/utils/TableGen/ClangASTPropertiesEmitter.cpp
> @@ -84,6 +84,7 @@ class ASTPropsEmitter {
>   raw_ostream 
>   RecordKeeper 
>   std::map NodeInfos;
> +  std::vector AllPropertyTypes;
> 
> public:
>   ASTPropsEmitter(RecordKeeper , raw_ostream )
> @@ -124,6 +125,15 @@ class ASTPropsEmitter {
>   info.Override = overrideRule;
> }
> 
> +for (PropertyType type :
> +   records.getAllDerivedDefinitions(PropertyTypeClassName)) {
> +  // Ignore generic specializations; they're generally not useful when
> +  // emitting basic emitters etc.
> +  if (type.isGenericSpecialization()) continue;
> +
> +  AllPropertyTypes.push_back(type);
> +}
> +
> Validator(*this).validate();
>   }
> 
> @@ -175,6 +185,11 @@ class ASTPropsEmitter {
>   void emitReadOfProperty(Property property);
>   void emitWriteOfProperty(Property property);
> 
> +  void emitBasicReaderWriterFile(const ReaderWriterInfo );
> +  void emitDispatcherTemplate(const ReaderWriterInfo );
> +  void emitPackUnpackOptionalTemplate(const ReaderWriterInfo );
> +  void emitBasicReaderWriterTemplate(const ReaderWriterInfo );
> +
> private:
>   class Validator {
> const ASTPropsEmitter 
> @@ -477,11 +492,11 @@ void clang::EmitClangTypeWriter(RecordKeeper , 
> raw_ostream ) {
> /*** BASIC READER/WRITERS ***/
> //
> 
> -static void emitDispatcherTemplate(ArrayRef types, raw_ostream ,
> -   const ReaderWriterInfo ) {
> +void
> +ASTPropsEmitter::emitDispatcherTemplate(const ReaderWriterInfo ) {
>   // Declare the {Read,Write}Dispatcher template.
>   StringRef dispatcherPrefix = (info.IsReader ? "Read" : "Write");
> -  out << "template \n"
> +  Out << "template \n"
>  "struct " << dispatcherPrefix << "Dispatcher;\n";
> 
>   // Declare a specific specialization of the dispatcher template.
> @@ -490,10 +505,10 @@ static void emitDispatcherTemplate(ArrayRef 
> types, raw_ostream ,
> const Twine ,
> StringRef methodSuffix) {
> StringRef var = info.HelperVariable;
> -out << "template " << specializationParameters << "\n"
> +Out << "template " << specializationParameters << "\n"
>"struct " << dispatcherPrefix << "Dispatcher<"
>  << cxxTypeName << "> {\n";
> -out << "  template  Args>\n"
> +Out << "  template  Args>\n"
>"  static " << (info.IsReader ? cxxTypeName : "void") << " "
><< info.MethodPrefix
><< "(Basic" << info.ClassSuffix << " &" << var
> @@ -506,7 +521,7 @@ static void emitDispatcherTemplate(ArrayRef 
> types, raw_ostream ,
>   };
> 
>   // Declare explicit specializations for each of the concrete types.
> -  for (PropertyType type : types) {
> +  for (PropertyType type : AllPropertyTypes) {
> declareSpecialization("<>",
>   type.getCXXTypeName(),
>   type.getAbstractTypeName());
> @@ -524,22 +539,21 @@ static void emitDispatcherTemplate(ArrayRef 
> types, raw_ostream ,
>   declareSpecialization("",
>   

[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Headers/opencl-c.h:14686
+#if defined(cl_khr_mipmap_image_writes)
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, 
float4 color);

Do we actually need pragma for this extension? I.e. does it need to activate 
any special mode in the compiler?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71460



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

[PATCH] D71476: [OpenCL] Add builtin function extension handling

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!




Comment at: clang/lib/Sema/OpenCLBuiltins.td:51
+// Extension associated to a builtin function.
+class FunctionExtension : AbstractExtension<_Ext>;
+

Are we planning to add type extensions too? If not it might not be worth 
creating extra abstractions. :)


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

https://reviews.llvm.org/D71476



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

[PATCH] D71500: [WebAssembly] Replace SIMD int min/max builtins with patterns

[Thomas Lively via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:189
+  setOperationAction(Op, T, Legal);
+}
+

aheejin wrote:
> Just curious, what gets assigned we don't do this? I thought the default 
> value was `Legal`..
When we don't do this, we get a sequence that does a compare and a select. 
You're right that the default is `Legal` for most things, but not for 
higher-level instructions like these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71500



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

I'll split the patch into 2 later




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

ABataev wrote:
> ABataev wrote:
> > CamelCase
> Seems to me, with mapper you're changing the contract of this function. It is 
> supposed to return the size in bytes, with Mapper it returns just number of 
> elements. Bad decision. Better to convert size in bytes to number of elements 
> in place where you need it.
Yes. The reason why I did this is I found this is the single place that I can 
consider all situations. Otherwise, I need to divide the size by the element 
size in other places. Since this function has discussed all situations for the 
element size, I will need to duplicate the work elsewhere if I don't do it 
here. The latter solution is not good for code mountainous I think.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7564-7573
   void generateInfoForComponentList(
-  OpenMPMapClauseKind MapType,
-  ArrayRef MapModifiers,
+  OpenMPMapClauseKind MapType, ArrayRef 
MapModifiers,
   OMPClauseMappableExprCommon::MappableExprComponentListRef Components,
   MapBaseValuesArrayTy , MapValuesArrayTy ,
   MapValuesArrayTy , MapFlagsArrayTy ,
-  StructRangeInfoTy , bool IsFirstComponentList,
-  bool IsImplicit,
+  MapMappersArrayTy , StructRangeInfoTy ,
+  bool IsFirstComponentList, bool IsImplicit,

ABataev wrote:
> Too many params in the function, worth thinking about wrapping them in a 
> record.
Maybe not in this patch?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8821-8822
+
+// No user-defined mapper for default mapping.
+CurMappers.push_back(nullptr);
   }

ABataev wrote:
> In general, there should not be branch for default mapping during the 
> codegen, the implicit map clauses must be generated in sema.
Not sure where this code will be used. I guess it's still correct to add a null 
to the mappers here?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8953-8955
+  llvm::Value *M = CGF.Builder.CreateConstInBoundsGEP2_32(
+  llvm::ArrayType::get(CGM.VoidPtrTy, Info.NumberOfPtrs),
+  Info.MappersArray, 0, I);

ABataev wrote:
> Better to use `Builder.CreateConstArrayGEP` 
I saw all the other places are using `CreateConstInBoundsGEP2_32` to get sizes, 
types, etc. So I kept it consistent here. Maybe these modifications should be 
in a future patch?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8956-8957
+  Info.MappersArray, 0, I);
+  M = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
+  M, MFunc->getType()->getPointerTo(/*AddrSpace=*/0));
+  Address MAddr(M, Ctx.getTypeAlignInChars(Ctx.VoidPtrTy));

ABataev wrote:
> Easier cast function to the `voidptr`, I think
Again, this is to keep it consistent with previous code above?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8991-8994
+  MappersArrayArg = CGF.Builder.CreateConstInBoundsGEP2_32(
+  llvm::ArrayType::get(CGM.VoidPtrTy, Info.NumberOfPtrs),
+  Info.MappersArray,
+  /*Idx0=*/0, /*Idx1=*/0);

ABataev wrote:
> Hmm, just cast of the `Info.MappersArray` to the correct type if all indices 
> are `0`?
Again, this is to keep consistent with previous code?


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

https://reviews.llvm.org/D67833



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 234113.
lildmh marked 28 inline comments as done.
lildmh added a comment.

Rebase and address comments


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

https://reviews.llvm.org/D67833

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/capturing_in_templates.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/declare_target_link_codegen.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_lambda_pointer_capturing.cpp
  clang/test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
  clang/test/OpenMP/openmp_offload_codegen.cpp
  clang/test/OpenMP/target_codegen.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  clang/test/OpenMP/target_defaultmap_codegen.cpp
  clang/test/OpenMP/target_depend_codegen.cpp
  clang/test/OpenMP/target_enter_data_codegen.cpp
  clang/test/OpenMP/target_enter_data_depend_codegen.cpp
  clang/test/OpenMP/target_exit_data_codegen.cpp
  clang/test/OpenMP/target_exit_data_depend_codegen.cpp
  clang/test/OpenMP/target_firstprivate_codegen.cpp
  clang/test/OpenMP/target_is_device_ptr_codegen.cpp
  clang/test/OpenMP/target_map_codegen.cpp
  clang/test/OpenMP/target_parallel_codegen.cpp
  clang/test/OpenMP/target_parallel_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_codegen.cpp
  clang/test/OpenMP/target_parallel_for_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_if_codegen.cpp
  clang/test/OpenMP/target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/target_simd_codegen.cpp
  clang/test/OpenMP/target_simd_depend_codegen.cpp
  clang/test/OpenMP/target_teams_codegen.cpp
  clang/test/OpenMP/target_teams_depend_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_collapse_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_depend_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_dist_schedule_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_collapse_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_dist_schedule_codegen.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_reduction_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_schedule_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_collapse_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  

[PATCH] D71500: [WebAssembly] Replace SIMD int min/max builtins with patterns

[Heejin Ahn via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:189
+  setOperationAction(Op, T, Legal);
+}
+

Just curious, what gets assigned we don't do this? I thought the default value 
was `Legal`..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71500



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