[PATCH] D71374: Improve support of GNU mempcpy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Please add a IR codegen test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

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

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 created this revision.
Jac1494 added reviewers: dblaikie, vsk, echristo.
Jac1494 added a project: debug-info.
Herald added subscribers: cfe-commits, hiraditya, aprantl.
Herald added projects: clang, LLVM.

Hi Devs,

Consider below testcases,

$cat shlib.c

int var;
int test()
{ return var++; }

$cat test.c

extern int test();
extern int var;
int main()
{ var++; printf("%d\n",test()); }

$clang  -fpic shlib.c -g -shared -o libshared.so
$clang  -L`pwd`   test.c -lshared -g
$export LD_LIBRARY_PATH=`pwd`

$ gdb a.out
GNU gdb (GDB) 8.2.50.20190204-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
http://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:

  .

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...
(gdb) b main
Breakpoint 1 at 0x400718: file test.c, line 6.
(gdb) pt var
type = 

As per above debugging ,for global extern variable "var" type info is missing 
because variable DebugInfo is not there in executable.
This is case where we don't run it, it means sharelib is not loaded. We can't 
get variable type.

LLVM is not adding debuginfo for externs with -g because this debug_info may 
increase final binary size.

So ,that i have given support for declaration of global extern variable with 
"-fstandalone-debug".

Thanks,
Jaydeep.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp


Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -171,7 +171,11 @@
   if (!GV->isDefinition())
 addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
+  {
+// Add location.
+addLocationAttribute(VariableDIE, GV, GlobalExprs);
 addGlobalName(GV->getName(), *VariableDIE, DeclContext);
+  }
 
   if (uint32_t AlignInBytes = GV->getAlignInBytes())
 addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
@@ -180,9 +184,6 @@
   if (MDTuple *TP = GV->getTemplateParams())
 addTemplateParams(*VariableDIE, DINodeArray(TP));
 
-  // Add location.
-  addLocationAttribute(VariableDIE, GV, GlobalExprs);
-
   return VariableDIE;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -171,7 +171,11 @@
   if (!GV->isDefinition())
 addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
+  {
+// Add location.
+addLocationAttribute(VariableDIE, GV, GlobalExprs);
 addGlobalName(GV->getName(), *VariableDIE, DeclContext);
+  }
 
   if (uint32_t AlignInBytes = GV->getAlignInBytes())
 addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
@@ -180,9 +184,6 @@
   if (MDTuple *TP = GV->getTemplateParams())
 addTemplateParams(*VariableDIE, DINodeArray(TP));
 
-  // Add location.
-  addLocationAttribute(VariableDIE, GV, GlobalExprs);
-
   return VariableDIE;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   

[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 233738.
void marked 3 inline comments as done.
void added a comment.

Use "any_of" and improve tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314

Files:
  clang/lib/Analysis/UninitializedValues.cpp
  clang/test/Analysis/uninit-asm-goto.cpp


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -Wuninitialized -verify %s
-// expected-no-diagnostics
 
+// test1: Expect no diagnostics
 int test1(int x) {
 int y;
 asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err);
@@ -8,3 +8,42 @@
   err:
 return -1;
 }
+
+int test2(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its 
declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  if (x < 42)
+asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) 
: "r"(x) :: indirect_1, indirect_2);
+  else
+asm volatile goto("testl %0, %1; testl %2, %3; jne %l5" : "+S"(x), "+D"(y) 
: "r"(x), "r"(y) :: indirect_1, indirect_2);
+  return x + y;
+indirect_1:
+  return -42;
+indirect_2:
+  return y; // expected-note {{uninitialized use occurs here}}
+}
+
+int test3(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its 
declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  asm goto("xorl %1, %0; jmp %l2" : "="(y) : "r"(x) : : fail);
+normal:
+  y += x;
+  return y;
+  if (x) {
+fail:
+return y; // expected-note {{uninitialized use occurs here}}
+  }
+  return 0;
+}
+
+int test4(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its 
declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  goto forward;
+backward:
+  return y; // expected-note {{uninitialized use occurs here}}
+forward:
+  asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : backward);
+  return y;
+}
Index: clang/lib/Analysis/UninitializedValues.cpp
===
--- clang/lib/Analysis/UninitializedValues.cpp
+++ clang/lib/Analysis/UninitializedValues.cpp
@@ -637,6 +637,26 @@
   continue;
 }
 
+if (AtPredExit == MayUninitialized) {
+  // If the predecessor's terminator is an "asm goto" that initializes
+  // the variable, then it won't be counted as "initialized" on the
+  // non-fallthrough paths.
+  CFGTerminator term = Pred->getTerminator();
+  if (GCCAsmStmt *as = dyn_cast_or_null(term.getStmt())) {
+if (as->isAsmGoto() &&
+llvm::any_of(as->outputs(), [&](const Expr *output) {
+return vd == findVar(output).getDecl() &&
+llvm::any_of(as->labels(),
+ [&](const AddrLabelExpr *label) {
+  return label->getLabel()->getStmt() == B->Label;
+});
+})) {
+  Use.setUninitAfterDecl();
+  continue;
+}
+  }
+}
+
 unsigned  = SuccsVisited[Pred->getBlockID()];
 if (!SV) {
   // When visiting the first successor of a block, mark all NULL
@@ -829,7 +849,8 @@
 
   for (const auto  : as->outputs())
 if (const VarDecl *VD = findVar(o).getDecl())
-  vals[VD] = Initialized;
+  if (vals[VD] != Initialized)
+vals[VD] = MayUninitialized;
 }
 
 void TransferFunctions::VisitObjCMessageExpr(ObjCMessageExpr *ME) {


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -Wuninitialized -verify %s
-// expected-no-diagnostics
 
+// test1: Expect no diagnostics
 int test1(int x) {
 int y;
 asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err);
@@ -8,3 +8,42 @@
   err:
 return -1;
 }
+
+int test2(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  if (x < 42)
+asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) : "r"(x) :: indirect_1, indirect_2);
+  else
+asm volatile goto("testl %0, %1; testl %2, %3; jne %l5" : "+S"(x), "+D"(y) : "r"(x), "r"(y) :: indirect_1, indirect_2);
+  return x + y;
+indirect_1:
+  return -42;
+indirect_2:
+  return y; // expected-note {{uninitialized use occurs here}}
+}
+
+int test3(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+ // expected-note {{initialize the 

[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/test/Analysis/uninit-asm-goto.cpp:16
+  if (x < 42)
+asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) 
: "r"(x) :: indirect_1, indirect_2);
+  else

nickdesaulniers wrote:
> I wonder if it's straight forward to make this a "maybe uninitialized" 
> warning, instead of "is uninitialized?"  Consider the inline asm:
> 
> ```
> asm volatile goto("ja %l1; mov %0, 42" : "=r"(foo) ::: bar);
> ```
> Since we can't introspect the inline asm, we're not sure whether `foo` gets 
> initialized by the asm or not (as the asm could transfer control flow back to 
> the C label before any assignments to the output).  Telling the user it's 
> definitely uninitialized is technically correct in this case, but definitely 
> incorrect for:
> 
> ```
> asm volatile goto("mov %0, 42; ja %l1;" : "=r"(foo) ::: bar);
> ```
The back end doesn't know how to generate code for a use in the indirect 
branches. It's really complicated and may result in code that doesn't actually 
work. I don't want to give off the impression that the code may work in these 
cases, because it would be essentially working by accident.



Comment at: clang/test/Analysis/uninit-asm-goto.cpp:38
+  return 0;
+}

nickdesaulniers wrote:
> Are we able to catch backwards branches from `asm goto`? (if so, would you 
> mind please added that as an additional unit test).
> 
> ```
> int foo;
> goto 1;
> 2:
> return foo; // should warn?
> 1:
> asm goto ("": "=r"(foo):::2);
> return foo;
> ```
Yes, we should be able to warn about this. I added a testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314



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

[PATCH] D71374: Improve support of GNU mempcpy

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

I am curious what is difference of code generation after applying your changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

Re: [PATCH] D71419: [clang] [test] Disable the test exhausting stack on NetBSD

[Eric Christopher via cfe-commits]

No worries, it happens.

On Thu, Dec 12, 2019, 10:59 PM Michał Górny  wrote:

> On Thu, 2019-12-12 at 14:47 -0800, Eric Christopher wrote:
> > As a quick note, this broke the test because you didn't update the line
> > numbers in the checks.
> >
> > Fixed thusly:
> >
> > echristo@athyra ~/r/llvm-project> git push
> > To github.com:llvm/llvm-project.git
> >6bed43f3c40..259a9b10390  master -> master
> >
> > :)
> >
>
> Thank you.  I'm sorry I didn't think that a such thing could happen.
>
> --
> Best regards,
> Michał Górny
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D71419: [clang] [test] Disable the test exhausting stack on NetBSD

[Michał Górny via cfe-commits]

On Thu, 2019-12-12 at 14:47 -0800, Eric Christopher wrote:
> As a quick note, this broke the test because you didn't update the line
> numbers in the checks.
> 
> Fixed thusly:
> 
> echristo@athyra ~/r/llvm-project> git push
> To github.com:llvm/llvm-project.git
>6bed43f3c40..259a9b10390  master -> master
> 
> :)
> 

Thank you.  I'm sorry I didn't think that a such thing could happen.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part
___
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#1782917 , @ABataev wrote:

> In D71241#1782898 , @JonChesterfield 
> wrote:
>
> > In D71241#1782846 , @ABataev wrote:
> >
> > > But I suggest to discuss this with Richard Smith.
> >
> >
> > Is the appeal to authority necessary to resolve this?
>
>
> Yes, necessary.


http://lists.llvm.org/pipermail/cfe-dev/2019-December/064101.html


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71241#1782668 , @ABataev wrote:

> In D71241#1782650 , @jdoerfert wrote:
>
> > While we talk a lot about what you think is bad about this solution it 
> > seems we ignore the problems in the current one. Let me summarize a few:
> >
> > - Take https://godbolt.org/z/XCjQUA where the wrong function is called in 
> > the target region (because the "hack" to inject code in the wrong 
> > definition is not applicable).
>
>
> No time for it, just short answers. No definition for the variant - no 
> definition for the base.


This is perfectly valid code and with the current scheme impossible to support.

>> - Take https://godbolt.org/z/Yi9Lht where the wrong function is called on 
>> the host (no there is *no* alias hidden)
> 
> GlobalAlias can be emitted only for definitions. No definition for variant - 
> no aliasing.

Exactly, as above, this is a problem.

>> - Take https://godbolt.org/z/2evvtN which shows that the alias solution is 
>> incompatible with linking.
> 
> Undefined behavior according to the standard.

I don't think so. If you do, please reference the rules this would violate.

>> - Take the `construct` context selector and the `begin/end declare variant` 
>> construct which both cannot be implemented with aliases.

This can also not be implemented in the alias scheme.


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] D71167: [Driver] Default to -momit-leaf-frame-pointer for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71167



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

[PATCH] D57732: Correct inf typo

[Jim Lin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4daa8d1de6dd: Correct inf typo (authored by gaul, committed 
by Jim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57732

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -981,7 +981,7 @@
 /// set, and the function could/should not be put on a single line (as per
 /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
 /// \code
-///   int f()   vs.   inf f()
+///   int f()   vs.   int f()
 ///   {}  {
 ///   }
 /// \endcode
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -958,7 +958,7 @@
 
 .. code-block:: c++
 
-  int f()   vs.   inf f()
+  int f()   vs.   int f()
   {}  {
   }
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -981,7 +981,7 @@
 /// set, and the function could/should not be put on a single line (as per
 /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
 /// \code
-///   int f()   vs.   inf f()
+///   int f()   vs.   int f()
 ///   {}  {
 ///   }
 /// \endcode
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -958,7 +958,7 @@
 
 .. code-block:: c++
 
-  int f()   vs.   inf f()
+  int f()   vs.   int f()
   {}  {
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4daa8d1 - Correct inf typo

[Jim Lin via cfe-commits]

Author: Andrew Gaul
Date: 2019-12-13T11:02:40+08:00
New Revision: 4daa8d1de6dda58aebfa7b19547ed3ce4e9bc91a

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

LOG: Correct inf typo

Reviewers: krasimir

Reviewed By: krasimir

Subscribers: Jim, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 2f7483435fd4..18cdc54a370f 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -958,7 +958,7 @@ the configuration (without a prefix: ``Auto``).
 
 .. code-block:: c++
 
-  int f()   vs.   inf f()
+  int f()   vs.   int f()
   {}  {
   }
 

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index f17a10c7f5c8..add2937f3b43 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -981,7 +981,7 @@ struct FormatStyle {
 /// set, and the function could/should not be put on a single line (as per
 /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
 /// \code
-///   int f()   vs.   inf f()
+///   int f()   vs.   int f()
 ///   {}  {
 ///   }
 /// \endcode



___
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

[kamlesh kumar via Phabricator via cfe-commits]

kamleshbhalui added a comment.

In D70537#1765607 , @probinson wrote:

> I guess first I'm confused about why the type would be undeduced in the first 
> place, given that it is actually instantiated.
>  And if undeduced is correct, wouldn't we rather emit these with 
> DW_TAG_unspecified_type?


While emitting the debug info this is the AST (for given testcase), in which 
static variable `value` is not yet deduced.
and when it tries to unwrap the type it fails.

  |-CXXRecordDecl 0x30dc88  col:8 implicit struct TypeId
  |-VarDecl 0x30dd30  col:23 used counter 'int' static inline 
listinit
  | `-InitListExpr 0x30ddd8  'int'
  |-VarTemplateDecl 0x30df68  col:30 identifier
  | |-TemplateTypeParmDecl 0x30de18  col:25 typename depth 0 index 0 
...
  | |-VarDecl 0x30df00  col:30 identifier 'const int':'const 
int' static inline cinit
  | | `-UnaryOperator 0x30dfe0  'int' postfix '++'
  | |   `-DeclRefExpr 0x30dfc0  'int' lvalue Var 0x30dd30 'counter' 
'int'
  | `-VarTemplateSpecializationDecl 0x341f30  col:30 referenced 
identifier 'const int':'const int' static inline cinit
  |   |-TemplateArgument pack
  |   | `-TemplateArgument type 'int'
  |   `-UnaryOperator 0x30dfe0  'int' postfix '++'
  | `-DeclRefExpr 0x30dfc0  'int' lvalue Var 0x30dd30 'counter' 
'int'
  |-VarTemplateDecl 0x30e228  col:30 value
  | |-TemplateTypeParmDecl 0x30e128  col:26 referenced 
typename depth 0 index 0 ... Args
  | |-VarDecl 0x30e1c0  col:30 value 'const auto' static 
inline cinit
  | | `-UnresolvedLookupExpr 0x30e2f8  '' 
lvalue (no ADL) = 'identifier' 0x30df68
  | `-VarTemplateSpecializationDecl 0x30e698  col:30 value 
'const auto' static inline
  |   `-TemplateArgument pack
  | `-TemplateArgument type 'int'
  |-VarTemplateSpecializationDecl 0x30e698  col:30 value 'const 
auto' static inline
  | `-TemplateArgument pack
  |   `-TemplateArgument type 'int'
  `-VarTemplateSpecializationDecl 0x341f30  col:30 referenced 
identifier 'const int':'const int' static inline cinit
|-TemplateArgument pack
| `-TemplateArgument type 'int'
`-UnaryOperator 0x30dfe0  'int' postfix '++'
  `-DeclRefExpr 0x30dfc0  'int' lvalue Var 0x30dd30 'counter' 'int'
   


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] D57732: Correct inf typo

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

I can commit it for you.
If you are interested in obtaining the commit right. you can refer 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57732



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

[clang] bc0c60f - Remove extra character I added to test my changes that I forgot to delete before submitting.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2019-12-12T18:23:08-08:00
New Revision: bc0c60f714fca54711b806c54467a8ce28c04181

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

LOG: Remove extra character I added to test my changes that I forgot to delete 
before submitting.

Added: 


Modified: 
clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp 
b/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
index 1286d744f0de..a48bb0edf51c 100644
--- a/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
+++ b/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
@@ -36,7 +36,7 @@ struct Y : Primary, X {
 };
 Y y;
 // ITANIUM: @_ZTV1Y = {{.*}}constant {{.*}} null, {{.*}} @_ZTI1Y {{.*}} 
@_ZN7Primary1fEv {{.*}} @_ZNK1YssERKS_ {{.*}} @_ZN1YaSERKS_ {{.*}} @_ZN1YaSEOS_ 
{{.*}} @_ZNK1YeqERKS_ {{.*}} -[[POINTERSIZE:4|8]]
-// ITANIUM-SAME: @_ZTI1Y {{.*}} @_ZThn[[POINTERSIZE]]_N1YaSERKS_a
+// ITANIUM-SAME: @_ZTI1Y {{.*}} @_ZThn[[POINTERSIZE]]_N1YaSERKS_
 
 struct A {
   void operator<=>(int);



___
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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782898 , @JonChesterfield 
wrote:

> In D71241#1782846 , @ABataev wrote:
>
> > But I suggest to discuss this with Richard Smith.
>
>
> Is the appeal to authority necessary to resolve this?


Yes, necessary.

> The last few posts by Hal look clear to me. Especially convincing is:
> 
>> We're simply resolving the callee according to the language rules.




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] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D71240#1782828 , @nridge wrote:

> In D71240#1782763 , @thakis wrote:
>
> > Probably needs the usual -fno-delayed-template-parsing workaround, see 
> > other clangd tests.
>
>
> Thanks for the suggestion! Fix at https://reviews.llvm.org/D71444


Committed in https://reviews.llvm.org/rG4f732a3d49ac.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D62731#1782897 , @andrew.w.kaylor 
wrote:

> In D62731#1782762 , @rjmccall wrote:
>
> > Currently we emit a warning if you use `-frounding-math`, saying that the 
> > option is ignored.  That should have alerted users that they're not getting 
> > the correct behavior now.  This patch removes the warning and (IIUC) 
> > implements the correct behavior but is over-conservative.  If that's 
> > correct, I think that's acceptable and we don't need an "experimental" flag 
> > or a warning.
>
>
> Oh, I didn't realize we were already warning about that. In theory, we should 
> handle rounding math correctly with this change. It's possible we've missed 
> some things, but I suppose that's always true. I think there are a few 
> general intrinsics left that need constrained versions but don't have them, 
> and we don't have any solution yet for target-specific intrinsics. If any of 
> those have special handling that assumes the default rounding mode we will 
> get it wrong. I don't think most users would be likely to encounter a problem.


Hmm.  The target-specific intrinsics thing does concern me, since I assume many 
targets have a bunch of vector intrinsics that may be sensitive to rounding.  
Do we have an idea of how we'd fix that?  If it's a short-term incorrectness 
that we have a plan to fix, I don't mind the risk, but if we don't know how 
we'd even start to address it...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

In D62731#1782762 , @rjmccall wrote:

> Currently we emit a warning if you use `-frounding-math`, saying that the 
> option is ignored.  That should have alerted users that they're not getting 
> the correct behavior now.  This patch removes the warning and (IIUC) 
> implements the correct behavior but is over-conservative.  If that's correct, 
> I think that's acceptable and we don't need an "experimental" flag or a 
> warning.


Oh, I didn't realize we were already warning about that. In theory, we should 
handle rounding math correctly with this change. It's possible we've missed 
some things, but I suppose that's always true. I think there are a few general 
intrinsics left that need constrained versions but don't have them, and we 
don't have any solution yet for target-specific intrinsics. If any of those 
have special handling that assumes the default rounding mode we will get it 
wrong. I don't think most users would be likely to encounter a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



___
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

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D71241#1782846 , @ABataev wrote:

> But I suggest to discuss this with Richard Smith.


Is the appeal to authority necessary to resolve this? The last few posts by Hal 
look clear to me. Especially convincing is:

> We're simply resolving the callee according to the language rules.


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] D71444: [clangd] Fix Windows test failure by adding -fno-delayed-template-parsing to LocateSymbol.Ambiguous

[Nathan Ridge via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f732a3d49ac: [clangd] Fix Windows test failure by adding 
-fno-delayed-template-parsing to… (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71444

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,7 +586,11 @@
   S::ba$13^z(u);
 }
   )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
+  auto TU = TestTU::withCode(T.code());
+  // FIXME: Go-to-definition in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+  auto AST = TU.build();
   // Ordered assertions are deliberate: we expect a predictable order.
   EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,7 +586,11 @@
   S::ba$13^z(u);
 }
   )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
+  auto TU = TestTU::withCode(T.code());
+  // FIXME: Go-to-definition in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+  auto AST = TU.build();
   // Ordered assertions are deliberate: we expect a predictable order.
   EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 4f732a3 - [clangd] Fix Windows test failure by adding -fno-delayed-template-parsing to LocateSymbol.Ambiguous

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2019-12-12T20:31:46-05:00
New Revision: 4f732a3d49ace980058bbb12150c8afc499af0ae

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

LOG: [clangd] Fix Windows test failure by adding -fno-delayed-template-parsing 
to LocateSymbol.Ambiguous

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 0352322790bd..51a7f81dcb5a 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,7 +586,11 @@ TEST(LocateSymbol, Ambiguous) {
   S::ba$13^z(u);
 }
   )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
+  auto TU = TestTU::withCode(T.code());
+  // FIXME: Go-to-definition in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+  auto AST = TU.build();
   // Ordered assertions are deliberate: we expect a predictable order.
   EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));



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

[clang] b71475f - Fix test to work correctly on 32-bit platforms.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2019-12-12T17:28:06-08:00
New Revision: b71475ff9ae02a8ec8d9841316c9717ef67b2f40

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

LOG: Fix test to work correctly on 32-bit platforms.

Added: 


Modified: 
clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp 
b/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
index e6be640a1f7f..1286d744f0de 100644
--- a/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
+++ b/clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
@@ -35,7 +35,8 @@ struct Y : Primary, X {
   virtual std::strong_ordering operator<=>(const Y&) const = default;
 };
 Y y;
-// ITANIUM: @_ZTV1Y = {{.*}}constant {{.*}} null, {{.*}} @_ZTI1Y {{.*}} 
@_ZN7Primary1fEv {{.*}} @_ZNK1YssERKS_ {{.*}} @_ZN1YaSERKS_ {{.*}} @_ZN1YaSEOS_ 
{{.*}} @_ZNK1YeqERKS_ {{.*}} -{{4|8}} {{.*}} @_ZTI1Y {{.*}} @_ZThn8_N1YaSERKS_
+// ITANIUM: @_ZTV1Y = {{.*}}constant {{.*}} null, {{.*}} @_ZTI1Y {{.*}} 
@_ZN7Primary1fEv {{.*}} @_ZNK1YssERKS_ {{.*}} @_ZN1YaSERKS_ {{.*}} @_ZN1YaSEOS_ 
{{.*}} @_ZNK1YeqERKS_ {{.*}} -[[POINTERSIZE:4|8]]
+// ITANIUM-SAME: @_ZTI1Y {{.*}} @_ZThn[[POINTERSIZE]]_N1YaSERKS_a
 
 struct A {
   void operator<=>(int);



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

[PATCH] D71447: [CIndex] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur created this revision.
Meinersbur added reviewers: abramobagnara, ddunbar, aganea, hfinkel, rjmccall, 
kbarton, SjoerdMeijer, aaron.ballman, ABataev, fhahn, hsaito, hans, greened, 
dmgreen, Ayal, asavonic, rtrieu, dorit, rsmith, tyler.nowicki, jdoerfert.
Herald added a subscriber: arphaman.
Herald added a project: clang.
Meinersbur added a parent revision: D69091: [Sema] #pragma clang transform.

Support for #pragma clang transform in cindex. Like for OpenMP directives, 
there is no CXCursor for clauses, such that expressions in clauses are visited 
directly as children of TransformExecutableDirective. CXCursor for clause might 
be implemented separately.

For a full description, see D69088 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71447

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/transform.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/tools/libclang/CursorVisitor.h

Index: clang/tools/libclang/CursorVisitor.h
===
--- clang/tools/libclang/CursorVisitor.h
+++ clang/tools/libclang/CursorVisitor.h
@@ -37,7 +37,8 @@
 MemberRefVisitKind,
 SizeOfPackExprPartsKind,
 LambdaExprPartsKind,
-PostChildrenVisitKind
+PostChildrenVisitKind,
+TransformClauseVisitKind
   };
 
 protected:
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -737,7 +737,8 @@
 K = CXCursor_BuiltinBitCastExpr;
 break;
   case Stmt::TransformExecutableDirectiveClass:
-llvm_unreachable("not implemented");
+K = CXCursor_TransformExecutableDirective;
+break;
   }
 
   CXCursor C = { K, 0, { Parent, S, TU } };
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2015,6 +2015,7 @@
   void VisitPseudoObjectExpr(const PseudoObjectExpr *E);
   void VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   void VisitLambdaExpr(const LambdaExpr *E);
+  void VisitTransformExecutableDirective(const TransformExecutableDirective *D);
   void VisitOMPExecutableDirective(const OMPExecutableDirective *D);
   void VisitOMPLoopDirective(const OMPLoopDirective *D);
   void VisitOMPParallelDirective(const OMPParallelDirective *D);
@@ -2750,6 +2751,19 @@
   Visit(E->getSyntacticForm());
 }
 
+void EnqueueVisitor::VisitTransformExecutableDirective(
+const TransformExecutableDirective *D) {
+  VisitStmt(D);
+
+  // Visit expressions used in clauses before the loop.
+  // TODO: Currently, the clauses have no CXCursor representation, such that
+  // they cannot be visited themselves. Therefore these expressions appear to be
+  // children of the TransformExecutableDirective.
+  for (const TransformClause *C : D->clauses())
+for (const Stmt *S : C->children())
+  AddStmt(S);
+}
+
 void EnqueueVisitor::VisitOMPExecutableDirective(
   const OMPExecutableDirective *D) {
   EnqueueChildren(D);
@@ -5558,6 +5572,8 @@
   return cxstring::createRef("attribute(warn_unused_result)");
   case CXCursor_AlignedAttr:
   return cxstring::createRef("attribute(aligned)");
+  case CXCursor_TransformExecutableDirective:
+return cxstring::createRef("TransformExecutableDirective");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
Index: clang/test/Index/transform.c
===
--- /dev/null
+++ clang/test/Index/transform.c
@@ -0,0 +1,16 @@
+// RUN: c-index-test -test-load-source local -Xclang -fexperimental-transform-pragma %s | FileCheck %s
+
+typedef int int_t;
+struct foo { long x; };
+
+void test() {
+#pragma clang transform unroll partial(2*2)
+  for (int i = 0; i < 8; i += 1)
+;
+}
+
+
+// CHECK: transform.c:7:15: TransformExecutableDirective= Extent=[7:15 - 7:44]
+// CHECK: transform.c:7:40: BinaryOperator= Extent=[7:40 - 7:43]
+// CHECK: transform.c:7:40: IntegerLiteral= Extent=[7:40 - 7:41]
+// CHECK: transform.c:7:42: IntegerLiteral= Extent=[7:42 - 7:43]
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -2574,7 +2574,11 @@
*/
   CXCursor_OMPParallelMasterDirective = 285,
 
-  CXCursor_LastStmt = CXCursor_OMPParallelMasterDirective,
+  /** Transformation directive.
+   */
+  CXCursor_TransformExecutableDirective = 286,
+
+  CXCursor_LastStmt = CXCursor_TransformExecutableDirective,
 
   /**
* Cursor that represents the translation unit itself.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71434: [Driver] Use .init_array for all gcc installations and simplify Generic_ELF -fno-use-init-array rules

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 233721.
MaskRay added a comment.

Add release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71434

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/constructors.c


Index: clang/test/Driver/constructors.c
===
--- clang/test/Driver/constructors.c
+++ clang/test/Driver/constructors.c
@@ -34,7 +34,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --gcc-toolchain="" \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -fuse-init-array \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2782,23 +2782,7 @@
 void Generic_ELF::addClangTargetOptions(const ArgList ,
 ArgStringList ,
 Action::OffloadKind) const {
-  const Generic_GCC::GCCVersion  = GCCInstallation.getVersion();
-  bool UseInitArrayDefault =
-  getTriple().getArch() == llvm::Triple::aarch64 ||
-  getTriple().getArch() == llvm::Triple::aarch64_be ||
-  (getTriple().isOSFreeBSD() &&
-   getTriple().getOSMajorVersion() >= 12) ||
-  (getTriple().getOS() == llvm::Triple::Linux &&
-   ((!GCCInstallation.isValid() || !V.isOlderThan(4, 7, 0)) ||
-getTriple().isAndroid())) ||
-  getTriple().getOS() == llvm::Triple::NaCl ||
-  (getTriple().getVendor() == llvm::Triple::MipsTechnologies &&
-   !getTriple().hasEnvironment()) ||
-  getTriple().getOS() == llvm::Triple::Solaris ||
-  getTriple().getArch() == llvm::Triple::riscv32 ||
-  getTriple().getArch() == llvm::Triple::riscv64;
-
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
-  options::OPT_fno_use_init_array, 
UseInitArrayDefault))
+  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fno-use-init-array");
 }
Index: clang/lib/Driver/ToolChains/FreeBSD.h
===
--- clang/lib/Driver/ToolChains/FreeBSD.h
+++ clang/lib/Driver/ToolChains/FreeBSD.h
@@ -76,6 +76,10 @@
   // Until dtrace (via CTF) and LLDB can deal with distributed debug info,
   // FreeBSD defaults to standalone/full debug info.
   bool GetDefaultStandaloneDebug() const override { return true; }
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
 
 protected:
   Tool *buildAssembler() const override;
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -467,3 +467,12 @@
 Res |= SanitizerKind::Memory;
   return Res;
 }
+
+void FreeBSD::addClangTargetOptions(const ArgList ,
+ArgStringList ,
+Action::OffloadKind) const {
+  if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
+  options::OPT_fno_use_init_array,
+  getTriple().getOSMajorVersion() >= 12))
+CC1Args.push_back("-fno-use-init-array");
+}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -89,6 +89,9 @@
   to run at a lower frequency which can impact performance. This behavior can 
be
   changed by passing -mprefer-vector-width=512 on the command line.
 
+* clang now defaults to ``.init_array`` on Linux. It used to use ``.ctors`` if
+  the found gcc installation is older than 4.7.0.
+
 New Compiler Flags
 --
 


Index: clang/test/Driver/constructors.c
===
--- clang/test/Driver/constructors.c
+++ clang/test/Driver/constructors.c
@@ -34,7 +34,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --gcc-toolchain="" \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -fuse-init-array \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782812 , @hfinkel wrote:

> In D71241#1782779 , @ABataev wrote:
>
> > In D71241#1782742 , @hfinkel wrote:
> >
> > > In D71241#1782703 , @ABataev 
> > > wrote:
> > >
> > > >
> > >
> > >
> > > ...
> > >
> > > >> 
> > > >> 
> > > >>> Given we have two implementations, each at different points in 
> > > >>> the pipeline, it might be constructive to each write down why you 
> > > >>> each choose said point. I suspect the rationale is hidden by the 
> > > >>> implementation details.
> > > > 
> > > > Actually, early resolution will break tbe tools, not help them. It 
> > > > will definitely break clangd, for example. The user will try to 
> > > > navigate to originally called function `base` and instead he will 
> > > > be redirected to the function `hst`.
> > >  
> > >  Can you please be more specific? Please explain why the user would 
> > >  consider this incorrect behavior. If the point of the tool is to 
> > >  allow the user to navigate to the function actually being called, 
> > >  then navigating to `base` seems incorrect if that's not the function 
> > >  being called. This is just like any other kind of overload 
> > >  resolution - the user likely wants to navigate to the function being 
> > >  called.
> > >  
> > >  Now the user might want an OpenMP-aware tool that understands 
> > >  differences between host and accelerator behavior, how that affects 
> > >  which functions are called, etc. The user might want this for 
> > >  host/device overloads in CUDA too, but this is really an orthogonal 
> > >  concern.
> > > >>> 
> > > >>> You wrote the code. You called a function in the expression. Now you 
> > > >>> want to navivate to this function. Clicked on it and instead of the 
> > > >>> called `base` you are redirected to `hst` because AST has the link to 
> > > >>> `hst` functiin inthe expression instead of the `base`.
> > > >> 
> > > >> Sure, but it has that link because that `hst` function is actually the 
> > > >> function being called (assuming that the clangd-using-tool is 
> > > >> configured to interpret the code as if compiling for the host). When I 
> > > >> click on a function call in a source file, I want to navigate to the 
> > > >> function actually being called. I certainly understand that the 
> > > >> function being called now depends on compilation context, and the tool 
> > > >> in our example is only providing the resolution in one context, but at 
> > > >> least it provides one valid answer. An OpenMP-aware tool could 
> > > >> navigate to the base function (we do need to preserve information to 
> > > >> make this possible). This is just like dealing with some host/device 
> > > >> functions in CUDA (where there are separate overloads) - if you click 
> > > >> on the function in such a tool you'll probably navigate to the host 
> > > >> variant of the function (even if, in some other context, the device 
> > > >> overload might be called).
> > > >> 
> > > >> Again, I see this as exactly analogous to overload resolution, or as 
> > > >> another example, when calling a function template with 
> > > >> specializations. When using such a tool, my experience is that users 
> > > >> want to click on the function and navigate to the function actually 
> > > >> being called. If, for example, I have a function template with 
> > > >> specializations, if the specialized one is being called, I should 
> > > >> navigate to the specialization being called (not the base function 
> > > >> template).
> > > > 
> > > > You wrote wrong context matcher. You has a bug in the base function, 
> > > > which should be called by default everu sw here but the host, and want 
> > > > to fix it. Etc.
> > >
> > > I understand, but this is a generic problem. Same with host/device 
> > > overloads in CUDA. Your tool only gets one compilation context, and thus 
> > > only one callee. In addition, FWIW, having a base version called 
> > > everywhere except on the host seems like an uncommon use case. Normally, 
> > > the base version *is* the version called on the host. The named variants 
> > > are likely the ones specialized for various accelerators.
> > >
> > > Regardless, this is exactly why we should do this in Sema. We can 
> > > represent links to both Decls in the AST (as I indicated in an earlier 
> > > comment), and then it will be *possible* for an OpenMP-aware tool to 
> > > decide on which it wants. Right now, it's not easily possible to write a 
> > > tool that can use an appropriate set of contexts to examine the AST where 
> > > the actual callees are represented.
> >
> >
> > No need to worry for the right decl. See D7097 
> > . If you see a refernce for function, 
> 

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D71240#1782763 , @thakis wrote:

> Probably needs the usual -fno-delayed-template-parsing workaround, see other 
> clangd tests.


Thanks for the suggestion! Fix at https://reviews.llvm.org/D71444


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240



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

[PATCH] D71444: [clangd] Fix Windows test failure by adding -fno-delayed-template-parsing to LocateSymbol.Ambiguous

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71444

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,7 +586,11 @@
   S::ba$13^z(u);
 }
   )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
+  auto TU = TestTU::withCode(T.code());
+  // FIXME: Go-to-definition in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+  auto AST = TU.build();
   // Ordered assertions are deliberate: we expect a predictable order.
   EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,7 +586,11 @@
   S::ba$13^z(u);
 }
   )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
+  auto TU = TestTU::withCode(T.code());
+  // FIXME: Go-to-definition in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+  auto AST = TU.build();
   // Ordered assertions are deliberate: we expect a predictable order.
   EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70032: [docs] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 233717.
Meinersbur added a comment.

- Add release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70032

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/PragmaClangTransform.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -19,6 +19,7 @@
UsersManual
Toolchain
LanguageExtensions
+   PragmaClangTransform
ClangCommandLineReference
AttributeReference
DiagnosticsReference
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -100,6 +100,10 @@
 - vzeroupper insertion on X86 targets can now be disabled with -mno-vzeroupper.
   You can also force vzeroupper insertion to be used on CPUs that normally
   wouldn't with -mvzeroupper.
+  
+- The -fexperimental-transform-pragma flag controls whether
+  ``#pragma clang transform`` pragmas are parsed. Otherwise, the they
+  are handled like an unknown pragma.
 
 Deprecated Compiler Flags
 -
@@ -122,7 +126,12 @@
 New Pragmas in Clang
 
 
-- ...
+- The ``#pragma clang transform`` directive has been added. The
+  directive instructs the compiler to apply code transformations.
+  Compared to ``#pragma clang loop``, it allows to chain multiple
+  transformations. See
+  `User-Directed Code Transformations 

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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1782779 , @ABataev wrote:

> In D71241#1782742 , @hfinkel wrote:
>
> > In D71241#1782703 , @ABataev wrote:
> >
> > >
> >
> >
> > ...
> >
> > >> 
> > >> 
> > >>> Given we have two implementations, each at different points in the 
> > >>> pipeline, it might be constructive to each write down why you each 
> > >>> choose said point. I suspect the rationale is hidden by the 
> > >>> implementation details.
> > > 
> > > Actually, early resolution will break tbe tools, not help them. It 
> > > will definitely break clangd, for example. The user will try to 
> > > navigate to originally called function `base` and instead he will be 
> > > redirected to the function `hst`.
> >  
> >  Can you please be more specific? Please explain why the user would 
> >  consider this incorrect behavior. If the point of the tool is to allow 
> >  the user to navigate to the function actually being called, then 
> >  navigating to `base` seems incorrect if that's not the function being 
> >  called. This is just like any other kind of overload resolution - the 
> >  user likely wants to navigate to the function being called.
> >  
> >  Now the user might want an OpenMP-aware tool that understands 
> >  differences between host and accelerator behavior, how that affects 
> >  which functions are called, etc. The user might want this for 
> >  host/device overloads in CUDA too, but this is really an orthogonal 
> >  concern.
> > >>> 
> > >>> You wrote the code. You called a function in the expression. Now you 
> > >>> want to navivate to this function. Clicked on it and instead of the 
> > >>> called `base` you are redirected to `hst` because AST has the link to 
> > >>> `hst` functiin inthe expression instead of the `base`.
> > >> 
> > >> Sure, but it has that link because that `hst` function is actually the 
> > >> function being called (assuming that the clangd-using-tool is configured 
> > >> to interpret the code as if compiling for the host). When I click on a 
> > >> function call in a source file, I want to navigate to the function 
> > >> actually being called. I certainly understand that the function being 
> > >> called now depends on compilation context, and the tool in our example 
> > >> is only providing the resolution in one context, but at least it 
> > >> provides one valid answer. An OpenMP-aware tool could navigate to the 
> > >> base function (we do need to preserve information to make this 
> > >> possible). This is just like dealing with some host/device functions in 
> > >> CUDA (where there are separate overloads) - if you click on the function 
> > >> in such a tool you'll probably navigate to the host variant of the 
> > >> function (even if, in some other context, the device overload might be 
> > >> called).
> > >> 
> > >> Again, I see this as exactly analogous to overload resolution, or as 
> > >> another example, when calling a function template with specializations. 
> > >> When using such a tool, my experience is that users want to click on the 
> > >> function and navigate to the function actually being called. If, for 
> > >> example, I have a function template with specializations, if the 
> > >> specialized one is being called, I should navigate to the specialization 
> > >> being called (not the base function template).
> > > 
> > > You wrote wrong context matcher. You has a bug in the base function, 
> > > which should be called by default everu sw here but the host, and want to 
> > > fix it. Etc.
> >
> > I understand, but this is a generic problem. Same with host/device 
> > overloads in CUDA. Your tool only gets one compilation context, and thus 
> > only one callee. In addition, FWIW, having a base version called everywhere 
> > except on the host seems like an uncommon use case. Normally, the base 
> > version *is* the version called on the host. The named variants are likely 
> > the ones specialized for various accelerators.
> >
> > Regardless, this is exactly why we should do this in Sema. We can represent 
> > links to both Decls in the AST (as I indicated in an earlier comment), and 
> > then it will be *possible* for an OpenMP-aware tool to decide on which it 
> > wants. Right now, it's not easily possible to write a tool that can use an 
> > appropriate set of contexts to examine the AST where the actual callees are 
> > represented.
>
>
> No need to worry for the right decl. See D7097 
> . If you see a refernce for function, before 
> doing something with it, just call member function 
> `getOpenMPDeclareVariantFunction()` and you get the function that must be 
> actually called here. The tool must do the same. That's hiw the tools work. 
> They must be aware of special costructs/attributes.


But 

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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1782723 , @ABataev wrote:

> I don't insist on function redefinition solution. You want to replace 
> functions - fine, but do this at the codegen, not in AST.


Again, no one is replacing anything, and we're not mutating the AST. We're 
simply resolving the callee according to the language rules. That's something 
that should be done during AST construction.

It's like if I have this code:

  template 
  int foo() { return 0; }
  
  template <>
  int foo<8>() { return 1; }
  
  int main() {
return foo<8>();
  }

and you said that, in the AST, it should look like the unspecialized `foo` was 
being called. And then later, in CodeGen, something happened in order to cause 
the correct specialization would be called. That clearly would not be 
considered an acceptable design.


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] D71408: [lit] Remove lit's REQUIRES-ANY directive

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Nice cleanup!


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] D69092: [CodeGen] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 233713.
Meinersbur added a comment.

- Remove handling of OpenMP simd and LoopHints by CGTransform


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69092

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCXX.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CGTransform.cpp
  clang/lib/CodeGen/CGTransform.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-distribute.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-interleave-factor.cpp
  
clang/test/CodeGenCXX/pragma-transform/pragma-transform-interleave-successor.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-interleave.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-unroll-full.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-unroll-partial.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-unroll-successor.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-unroll.cpp
  
clang/test/CodeGenCXX/pragma-transform/pragma-transform-unrollandjam-partial.cpp
  
clang/test/CodeGenCXX/pragma-transform/pragma-transform-unrollandjam-predecessor.cpp
  
clang/test/CodeGenCXX/pragma-transform/pragma-transform-unrollandjam-successor.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-unrollandjam.cpp
  
clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize-successor.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize-width.cpp
  clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize.cpp

Index: clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fexperimental-transform-pragma -emit-llvm -o - %s | FileCheck %s
+
+extern "C" void pragma_transform_vectorize(int *List, int Length) {
+#pragma clang transform vectorize
+  for (int i = 0; i < Length; i++) {
+// CHECK: br label %{{.*}}, !llvm.loop ![[LOOP:[0-9]+]]
+List[i] = i * 2;
+  }
+}
+
+
+// CHECK-DAG: ![[LOOP]] = distinct !{![[LOOP]], ![[VECTORIZE_ENABLE:[0-9]+]], ![[INTERLEAVE_DISABLE:[0-9]+]], ![[VECTORIZE_FOLLOWUP_ALL:[0-9]+]], ![[DISABLE_NONFORCED:[0-9]+]]}
+// CHECK-DAG: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK-DAG: ![[INTERLEAVE_DISABLE]] = !{!"llvm.loop.interleave.count", i32 1}
+
+// CHECK-DAG: ![[VECTORIZE_FOLLOWUP_ALL]] = !{!"llvm.loop.vectorize.followup_all", ![[LOOP_VECTORIZE_ALL:[0-9]+]]}
+// CHECK-DAG: ![[DISABLE_NONFORCED]] = !{!"llvm.loop.disable_nonforced"}
+
+// CHECK-DAG: ![[LOOP_VECTORIZE_ALL]] = distinct !{![[LOOP_VECTORIZE_ALL]], ![[DISABLE_NONFORCED:[0-9]+]]}
Index: clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize-width.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize-width.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fexperimental-transform-pragma -emit-llvm -o - %s | FileCheck %s
+
+extern "C" void pragma_transform_vectorize(int *List, int Length) {
+#pragma clang transform vectorize width(4)
+  for (int i = 0; i < Length; i++) {
+// CHECK: br label %{{.*}}, !llvm.loop ![[LOOP:[0-9]+]]
+List[i] = i * 2;
+  }
+}
+
+
+// CHECK-DAG: ![[LOOP]] = distinct !{![[LOOP]], ![[VECTORIZE_ENABLE:[0-9]+]], ![[INTERLEAVE_DISABLE:[0-9]+]], ![[VECTORIZE_WIDTH:[0-9]+]], ![[VECTORIZE_FOLLOWUP_ALL:[0-9]+]], ![[DISABLE_NONFORCED:[0-9]+]]}
+// CHECK-DAG: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK-DAG: ![[INTERLEAVE_DISABLE]] = !{!"llvm.loop.interleave.count", i32 1}
+// CHECK-DAG: ![[VECTORIZE_WIDTH]] = !{!"llvm.loop.vectorize.width", i32 4}
+
+// CHECK-DAG: ![[VECTORIZE_FOLLOWUP_ALL]] = !{!"llvm.loop.vectorize.followup_all", ![[LOOP_VECTORIZE_ALL:[0-9]+]]}
+// CHECK-DAG: ![[DISABLE_NONFORCED]] = !{!"llvm.loop.disable_nonforced"}
+
+// CHECK-DAG: ![[LOOP_VECTORIZE_ALL]] = distinct !{![[LOOP_VECTORIZE_ALL]], ![[DISABLE_NONFORCED:[0-9]+]]}
Index: clang/test/CodeGenCXX/pragma-transform/pragma-transform-vectorize-successor.cpp
===
--- /dev/null
+++ 

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

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 233712.
thopre added a comment.
Herald added subscribers: Sanitizers, cfe-commits, luismarques, apazos, 
sameer.abuasal, pzheng, s.egerton, lenary, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, delcypher, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb.
Herald added projects: clang, Sanitizers.

Remove REQUIRES-ANY lit directive


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408

Files:
  clang/test/Driver/XRay/xray-instrument-macos.c
  clang/test/Driver/XRay/xray-instrument-os.c
  clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
  clang/test/Driver/XRay/xray-mode-flags.cpp
  clang/test/Driver/XRay/xray-nolinkdeps.cpp
  compiler-rt/test/builtins/Unit/arm/aeabi_cdcmpeq_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cdcmple_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cfcmpeq_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cfcmple_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_drsub_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_frsub_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_idivmod_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
  compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
  llvm/utils/lit/lit/TestRunner.py
  llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
  llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt

Index: llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt
===
--- llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-RUN: true
-REQUIRES-ANY: a-missing-feature, a-present-feature
Index: llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
===
--- llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-RUN: true
-REQUIRES-ANY: a-missing-feature, a-missing-feature-2
Index: llvm/utils/lit/lit/TestRunner.py
===
--- llvm/utils/lit/lit/TestRunner.py
+++ llvm/utils/lit/lit/TestRunner.py
@@ -1304,20 +1304,6 @@
 BooleanExpression.evaluate(s, [])
 return output
 
-@staticmethod
-def _handleRequiresAny(line_number, line, output):
-"""A custom parser to transform REQUIRES-ANY: into REQUIRES:"""
-
-# Extract the conditions specified in REQUIRES-ANY: as written.
-conditions = []
-IntegratedTestKeywordParser._handleList(line_number, line, conditions)
-
-# Output a `REQUIRES: a || b || c` expression in its place.
-expression = ' || '.join(conditions)
-IntegratedTestKeywordParser._handleBooleanExpr(line_number,
-   expression, output)
-return output
-
 def parseIntegratedTestScript(test, additional_parsers=[],
   require_script=True):
 """parseIntegratedTestScript - Scan an LLVM/Clang style integrated test
@@ -1341,9 +1327,6 @@
 initial_value=test.xfails),
 IntegratedTestKeywordParser('REQUIRES:', ParserKind.BOOLEAN_EXPR,
 initial_value=test.requires),
-IntegratedTestKeywordParser('REQUIRES-ANY:', ParserKind.CUSTOM,
-IntegratedTestKeywordParser._handleRequiresAny, 
-initial_value=test.requires), 
 IntegratedTestKeywordParser('UNSUPPORTED:', ParserKind.BOOLEAN_EXPR,
 initial_value=test.unsupported),
 IntegratedTestKeywordParser('END.', ParserKind.TAG)
Index: compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
===
--- compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
+++ compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
@@ -1,4 +1,4 @@
-// REQUIRES-ANY: riscv32-target-arch
+// REQUIRES: riscv32-target-arch
 // RUN: %clang_builtins %s %librt -o %t && %run %t
 //===-- mulsi3_test.c - Test __mulsi3 -===//
 //
Index: compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
===
--- compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
+++ compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
@@ -1,4 +1,4 @@
-// REQUIRES-ANY: arm-target-arch,armv6m-target-arch
+// REQUIRES: arm-target-arch || armv6m-target-arch
 // RUN: %clang_builtins %s %librt -o %t && %run %t
 //===-- aeabi_uldivmod_test.c - Test aeabi_uldivmod ---===//
 //
Index: compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c

[PATCH] D70572: [Serialization] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 233714.
Meinersbur added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70572

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/PCH/transform-distribute.cpp
  clang/test/PCH/transform-interleave.cpp
  clang/test/PCH/transform-unroll.cpp
  clang/test/PCH/transform-unrollandjam.cpp
  clang/test/PCH/transform-vectorize.cpp

Index: clang/test/PCH/transform-vectorize.cpp
===
--- /dev/null
+++ clang/test/PCH/transform-vectorize.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -include-pch %t.pch %s -ast-dump-all -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+void vectorize_heuristic(int n) {
+#pragma clang transform vectorize
+  for (int i = 0; i < n; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported vectorize_heuristic
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: ForStmt
+
+
+void vectorize_width(int n) {
+#pragma clang transform vectorize width(4)
+  for (int i = 0; i < n; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported vectorize_width
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: WidthClause
+// CHECK-NEXT:   IntegerLiteral {{.*}} 'int' 4
+// CHECK-NEXT: ForStmt
+
+#endif /* HEADER */
Index: clang/test/PCH/transform-unrollandjam.cpp
===
--- /dev/null
+++ clang/test/PCH/transform-unrollandjam.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -include-pch %t.pch %s -ast-dump-all -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+void  unrollandjam_heuristic(int n) {
+#pragma clang transform unrollandjam
+  for (int i = 0; i < n; i+=1)
+for (int j = 0; j < n; j+=1)
+  ;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unrollandjam_heuristic
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: ForStmt
+
+
+void unrollandjam_partial(int n) {
+#pragma clang transform unrollandjam partial(4)
+  for (int i = 0; i < n; i+=1)
+for (int j = 0; j < n; j+=1)
+  ;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unrollandjam_partial
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: PartialClause
+// CHECK-NEXT:   IntegerLiteral {{.*}} 'int' 4
+// CHECK-NEXT: ForStmt
+
+#endif /* HEADER */
Index: clang/test/PCH/transform-unroll.cpp
===
--- /dev/null
+++ clang/test/PCH/transform-unroll.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexperimental-transform-pragma -include-pch %t.pch %s -ast-dump-all -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+void  unroll_heuristic(int n) {
+#pragma clang transform unroll
+  for (int i = 0; i < 4; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unroll_heuristic
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: ForStmt
+
+
+void unroll_full(int n) {
+#pragma clang transform unroll full
+  for (int i = 0; i < 4; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unroll_full
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: FullClause
+// CHECK-NEXT: ForStmt
+
+
+void unroll_partial(int n) {
+#pragma clang transform unroll partial(4)
+  for (int i = 0; i < n; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unroll_partial
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: PartialClause
+// CHECK-NEXT:   IntegerLiteral {{.*}} 'int' 4
+// CHECK-NEXT: ForStmt
+
+
+template
+void unroll_template_function(int n) {
+#pragma clang transform unroll partial(FACTOR)
+  for (int i = 0; i < n; i+=1)
+;
+}
+// CHECK-LABEL: FunctionDecl {{.*}} imported unroll_template_function
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: PartialClause
+// CHECK-NEXT:   DeclRefExpr {{.*}} 'FACTOR' 'int'
+// CHECK-NEXT: ForStmt
+
+
+template void unroll_template_function<5>(int);
+// CHECK-LABEL: FunctionDecl {{.*}} imported unroll_template_function
+// CHECK: TransformExecutableDirective
+// CHECK-NEXT: PartialClause
+// CHECK-NEXT:   SubstNonTypeTemplateParmExpr
+// CHECK-NEXT:   IntegerLiteral {{.*}} 'int' 5
+// CHECK-NEXT: ForStmt
+
+
+template
+struct Unroll {
+  

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782742 , @hfinkel wrote:

> In D71241#1782703 , @ABataev wrote:
>
> >
>
>
> ...
>
> >> 
> >> 
> >>> Given we have two implementations, each at different points in the 
> >>> pipeline, it might be constructive to each write down why you each 
> >>> choose said point. I suspect the rationale is hidden by the 
> >>> implementation details.
> > 
> > Actually, early resolution will break tbe tools, not help them. It will 
> > definitely break clangd, for example. The user will try to navigate to 
> > originally called function `base` and instead he will be redirected to 
> > the function `hst`.
>  
>  Can you please be more specific? Please explain why the user would 
>  consider this incorrect behavior. If the point of the tool is to allow 
>  the user to navigate to the function actually being called, then 
>  navigating to `base` seems incorrect if that's not the function being 
>  called. This is just like any other kind of overload resolution - the 
>  user likely wants to navigate to the function being called.
>  
>  Now the user might want an OpenMP-aware tool that understands 
>  differences between host and accelerator behavior, how that affects 
>  which functions are called, etc. The user might want this for 
>  host/device overloads in CUDA too, but this is really an orthogonal 
>  concern.
> >>> 
> >>> You wrote the code. You called a function in the expression. Now you want 
> >>> to navivate to this function. Clicked on it and instead of the called 
> >>> `base` you are redirected to `hst` because AST has the link to `hst` 
> >>> functiin inthe expression instead of the `base`.
> >> 
> >> Sure, but it has that link because that `hst` function is actually the 
> >> function being called (assuming that the clangd-using-tool is configured 
> >> to interpret the code as if compiling for the host). When I click on a 
> >> function call in a source file, I want to navigate to the function 
> >> actually being called. I certainly understand that the function being 
> >> called now depends on compilation context, and the tool in our example is 
> >> only providing the resolution in one context, but at least it provides one 
> >> valid answer. An OpenMP-aware tool could navigate to the base function (we 
> >> do need to preserve information to make this possible). This is just like 
> >> dealing with some host/device functions in CUDA (where there are separate 
> >> overloads) - if you click on the function in such a tool you'll probably 
> >> navigate to the host variant of the function (even if, in some other 
> >> context, the device overload might be called).
> >> 
> >> Again, I see this as exactly analogous to overload resolution, or as 
> >> another example, when calling a function template with specializations. 
> >> When using such a tool, my experience is that users want to click on the 
> >> function and navigate to the function actually being called. If, for 
> >> example, I have a function template with specializations, if the 
> >> specialized one is being called, I should navigate to the specialization 
> >> being called (not the base function template).
> > 
> > You wrote wrong context matcher. You has a bug in the base function, which 
> > should be called by default everu sw here but the host, and want to fix it. 
> > Etc.
>
> I understand, but this is a generic problem. Same with host/device overloads 
> in CUDA. Your tool only gets one compilation context, and thus only one 
> callee. In addition, FWIW, having a base version called everywhere except on 
> the host seems like an uncommon use case. Normally, the base version *is* the 
> version called on the host. The named variants are likely the ones 
> specialized for various accelerators.
>
> Regardless, this is exactly why we should do this in Sema. We can represent 
> links to both Decls in the AST (as I indicated in an earlier comment), and 
> then it will be *possible* for an OpenMP-aware tool to decide on which it 
> wants. Right now, it's not easily possible to write a tool that can use an 
> appropriate set of contexts to examine the AST where the actual callees are 
> represented.


No need to worry for the right decl. See D7097 
. If you see a refernce for function, before 
doing something with it, just call member function 
`getOpenMPDeclareVariantFunction()` and you get the function that must be 
actually called here. The tool must do the same. That's hiw the tools work. 
They must be aware of special costructs/attributes.


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

[PATCH] D69091: [Sema] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 233707.
Meinersbur added a comment.

- Remove handling of LoopHints (#pragma clang loop) and OpenMP (#pragma omp 
simd) to reducethe size of the patches. Compatibility with OpenMP is still 
a goal (to implement the upcoming OpenMP loop transformations such as #pragma 
omp tile), but for now they are handled by separate code paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69091

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtTransform.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Analysis/AnalysisTransform.h
  clang/include/clang/Analysis/TransformedTree.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaTransform.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/StmtTransform.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTransform.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-transform-unroll.c
  clang/test/AST/ast-dump-transform-unrollandjam.c
  clang/test/AST/ast-print-pragma-transform-distribute.cpp
  clang/test/AST/ast-print-pragma-transform-interleave.cpp
  clang/test/AST/ast-print-pragma-transform-unroll.cpp
  clang/test/AST/ast-print-pragma-transform-unrollandjam.cpp
  clang/test/AST/ast-print-pragma-transform-vectorize.cpp
  clang/test/SemaCXX/pragma-transform-interleave.cpp
  clang/test/SemaCXX/pragma-transform-legacymix.cpp
  clang/test/SemaCXX/pragma-transform-unroll.cpp
  clang/test/SemaCXX/pragma-transform-unrollandjam.cpp
  clang/test/SemaCXX/pragma-transform-vectorize.cpp
  clang/test/SemaCXX/pragma-transform-wrongorder.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -735,6 +735,9 @@
 break;
   case Stmt::BuiltinBitCastExprClass:
 K = CXCursor_BuiltinBitCastExpr;
+break;
+  case Stmt::TransformExecutableDirectiveClass:
+llvm_unreachable("not implemented");
   }
 
   CXCursor C = { K, 0, { Parent, S, TU } };
Index: clang/test/SemaCXX/pragma-transform-wrongorder.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pragma-transform-wrongorder.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 -fexperimental-transform-pragma -fsyntax-only -verify %s
+
+void wrongorder(int *List, int Length, int Value) {
+
+/* expected-warning@+1 {{the LLVM pass structure currently is not able to apply the transformations in this order}} */
+#pragma clang transform distribute
+#pragma clang transform vectorize
+  for (int i = 0; i < 8; i++)
+List[i] = Value;
+
+/* expected-warning@+1 {{the LLVM pass structure currently is not able to apply the transformations in this order}} */
+#pragma clang transform vectorize
+#pragma clang transform unrollandjam
+  for (int i = 0; i < 8; i++)
+for (int j = 0; j < 8; j++)
+  List[i] += j;
+
+}
Index: clang/test/SemaCXX/pragma-transform-vectorize.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pragma-transform-vectorize.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fexperimental-transform-pragma -fsyntax-only -verify %s
+
+void vectorize(int *List, int Length, int Value) {
+/* expected-error@+1 {{the width clause can be specified at most once}} */
+#pragma clang transform vectorize width(4) width(4)
+  for (int i = 0; i < Length; i++)
+  List[i] = Value;
+
+/* expected-error@+1 {{clause argument must me at least 2}} */
+#pragma clang transform vectorize width(-42)
+  for (int i = 0; i < Length; i++)
+List[i] = Value;
+
+}
Index: clang/test/SemaCXX/pragma-transform-unrollandjam.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pragma-transform-unrollandjam.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c++11 -fexperimental-transform-pragma -fsyntax-only -verify %s
+
+void unrollandjam(int *List, int Length, int Value) {
+/* expected-error@+1 {{the partial clause can be specified at most once}} */
+#pragma clang transform unrollandjam partial(4) partial(4)
+  for (int i = 0; i < Length; i++)
+for (int j = 0; j < Length; j++)
+  List[i] += j*Value;
+
+/* expected-error@+1 {{unroll-and-jam requires exactly one nested loop}} */
+#pragma clang transform unrollandjam
+  for (int i = 

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

[Reid Kleckner via Phabricator via cfe-commits]

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

Seems reasonable, looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71439



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Currently we emit a warning if you use `-frounding-math`, saying that the 
option is ignored.  That should have alerted users that they're not getting the 
correct behavior now.  This patch removes the warning and (IIUC) implements the 
correct behavior but is over-conservative.  If that's correct, I think that's 
acceptable and we don't need an "experimental" flag or a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Tests fail on Windows: http://45.33.8.238/win/3976/step_9.txt

Probably needs the usual -fno-delayed-template-parsing workaround, see other 
clangd tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240



___
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#1782703 , @ABataev wrote:

>


...

>> 
>> 
>>> Given we have two implementations, each at different points in the 
>>> pipeline, it might be constructive to each write down why you each 
>>> choose said point. I suspect the rationale is hidden by the 
>>> implementation details.
> 
> Actually, early resolution will break tbe tools, not help them. It will 
> definitely break clangd, for example. The user will try to navigate to 
> originally called function `base` and instead he will be redirected to 
> the function `hst`.
 
 Can you please be more specific? Please explain why the user would 
 consider this incorrect behavior. If the point of the tool is to allow the 
 user to navigate to the function actually being called, then navigating to 
 `base` seems incorrect if that's not the function being called. This is 
 just like any other kind of overload resolution - the user likely wants to 
 navigate to the function being called.
 
 Now the user might want an OpenMP-aware tool that understands differences 
 between host and accelerator behavior, how that affects which functions 
 are called, etc. The user might want this for host/device overloads in 
 CUDA too, but this is really an orthogonal concern.
>>> 
>>> You wrote the code. You called a function in the expression. Now you want 
>>> to navivate to this function. Clicked on it and instead of the called 
>>> `base` you are redirected to `hst` because AST has the link to `hst` 
>>> functiin inthe expression instead of the `base`.
>> 
>> Sure, but it has that link because that `hst` function is actually the 
>> function being called (assuming that the clangd-using-tool is configured to 
>> interpret the code as if compiling for the host). When I click on a function 
>> call in a source file, I want to navigate to the function actually being 
>> called. I certainly understand that the function being called now depends on 
>> compilation context, and the tool in our example is only providing the 
>> resolution in one context, but at least it provides one valid answer. An 
>> OpenMP-aware tool could navigate to the base function (we do need to 
>> preserve information to make this possible). This is just like dealing with 
>> some host/device functions in CUDA (where there are separate overloads) - if 
>> you click on the function in such a tool you'll probably navigate to the 
>> host variant of the function (even if, in some other context, the device 
>> overload might be called).
>> 
>> Again, I see this as exactly analogous to overload resolution, or as another 
>> example, when calling a function template with specializations. When using 
>> such a tool, my experience is that users want to click on the function and 
>> navigate to the function actually being called. If, for example, I have a 
>> function template with specializations, if the specialized one is being 
>> called, I should navigate to the specialization being called (not the base 
>> function template).
> 
> You wrote wrong context matcher. You has a bug in the base function, which 
> should be called by default everu sw here but the host, and want to fix it. 
> Etc.

I understand, but this is a generic problem. Same with host/device overloads in 
CUDA. Your tool only gets one compilation context, and thus only one callee. In 
addition, FWIW, having a base version called everywhere except on the host 
seems like an uncommon use case. Normally, the base version *is* the version 
called on the host. The named variants are likely the ones specialized for 
various accelerators.

Regardless, this is exactly why we should do this in Sema. We can represent 
links to both Decls in the AST (as I indicated in an earlier comment), and then 
it will be *possible* for an OpenMP-aware tool to decide on which it wants. 
Right now, it's not easily possible to write a tool that can use an appropriate 
set of contexts to examine the AST where the actual callees are represented.


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] D71039: Add support for the MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Sorry for the delay, overall this seems like the right approach.




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

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.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:1874-1881
+case LangAS::ptr32_sptr:
+  Extra.mangleSourceName("_ASPtr32_sptr");
+  break;
+case LangAS::ptr32_uptr:
+  Extra.mangleSourceName("_ASPtr32_uptr");
+  break;
+case LangAS::ptr64:

This code should be unreachable because you check for these address spaces at 
the call site. I think you can do something like this:
  case LangAS::...:
  case LangAS::...:
  case LangAS::...:
llvm_unreachable("don't mangle ptr address spaces with _AS");



Comment at: clang/lib/AST/TypePrinter.cpp:1824
+  case LangAS::ptr32_sptr:
+OS << "__ptr32_sptr";
+break;

Think we should say `"__sptr __ptr32"`? This code doesn't guarantee that it can 
be parsed back as valid source, but it's closer.



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

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.



Comment at: clang/lib/Sema/SemaOverload.cpp:2890
 
+static QualType RemovePtrSizeAddrSpace(ASTContext , QualType T) {
+  if (const PointerType *Ptr = T->getAs()) {

I think it would be fair to raise this method up to ASTContext, next to 
getAddrSpaceQualType, removeAddrSpaceQualType, etc.


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] D69089: [Parser] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 233704.
Meinersbur added a comment.

- Simplify transformation classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69089

Files:
  clang/include/clang/AST/StmtTransform.h
  clang/include/clang/AST/TransformClauseKinds.def
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Transform.h
  clang/include/clang/Basic/TransformKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/StmtTransform.cpp
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Transform.cpp
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTransform.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaTransform.cpp
  clang/test/Parser/pragma-transform.cpp

Index: clang/test/Parser/pragma-transform.cpp
===
--- /dev/null
+++ clang/test/Parser/pragma-transform.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -std=c++11 -fexperimental-transform-pragma -verify %s
+
+void pragma_transform(int *List, int Length) {
+// FIXME: This does not emit an error
+#pragma clang
+
+/* expected-error@+1 {{expected a transformation name}} */
+#pragma clang transform
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{unknown transformation}} */
+#pragma clang transform unknown_transformation
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+2 {{expected loop after transformation pragma}} */
+#pragma clang transform unroll
+  pragma_transform(List, Length);
+
+/* expected-error@+1 {{unknown clause name}} */
+#pragma clang transform unroll unknown_clause
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{expected '(' after 'partial'}} */
+#pragma clang transform unroll partial
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{expected expression}} */
+#pragma clang transform unroll partial(
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{expected '(' after 'partial'}} */
+#pragma clang transform unroll partial)
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+2 {{expected ')'}} */
+/* expected-note@+1 {{to match this '('}} */
+#pragma clang transform unroll partial(4
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{expected expression}} */
+#pragma clang transform unroll partial()
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+/* expected-error@+1 {{use of undeclared identifier 'badvalue'}} */
+#pragma clang transform unroll partial(badvalue)
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+
+  {
+/* expected-error@+2 {{expected statement}} */
+#pragma clang transform unroll
+  }
+}
+
+/* expected-error@+1 {{expected unqualified-id}} */
+#pragma clang transform unroll
+int I;
+
+/* expected-error@+1 {{expected unqualified-id}} */
+#pragma clang transform unroll
+void func();
+
+class C1 {
+/* expected-error@+3 {{this pragma cannot appear in class declaration}} */
+/* expected-error@+2 {{expected member name or ';' after declaration specifiers}} */
+/* expected-error@+1 {{unknown type name 'unroll'}} */
+#pragma clang transform unroll
+};
+
+template
+void pragma_transform_template_func(int *List, int Length) {
+#pragma clang transform unroll partial(F)
+  for (int i = 0; i < Length; i+=1)
+  List[i] = i;
+}
+
+template
+class C2 {
+  void pragma_transform_template_class(int *List, int Length) {
+#pragma clang transform unroll partial(F)
+for (int i = 0; i < Length; i+=1)
+List[i] = i;
+  }
+};
Index: clang/lib/Sema/SemaTransform.cpp
===
--- /dev/null
+++ clang/lib/Sema/SemaTransform.cpp
@@ -0,0 +1,49 @@
+//=== SemaTransform.h - -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Semantic analysis for code transformations.
+//
+//===--===//
+
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/StmtTransform.h"
+#include "clang/Basic/Transform.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
+
+using namespace clang;
+
+StmtResult
+Sema::ActOnLoopTransformDirective(Transform::Kind Kind,
+  llvm::ArrayRef Clauses,
+  Stmt *AStmt, SourceRange 

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
 
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+  "Support for floating point control option %0 is incomplete and 
experimental">,

andrew.w.kaylor wrote:
> rupprecht wrote:
> > mibintc wrote:
> > > @kpn thought it would be a good idea to add a Warning that the 
> > > implementation of float control is experimental and partially 
> > > implemented.  That's what this is for.
> > Instead of adding a warning, I'd like to propose `-frounding-math` not be 
> > enabled unless an additional flag (e.g. `-fexperimental-float-control`) is 
> > passed. Or maybe this feature should be called 
> > `-f[no-]experimental-rounding-math` instead.
> > 
> > There are plenty of builds that are already specifying `-frounding-math` 
> > (e.g. they also support building w/ a compiler such as gcc that implements 
> > this), and adding this experimental/incomplete implementation is a surprise 
> > to those builds.
> > 
> > If I'm wrong and it's completely safe to ignore the warning (maybe it's 
> > incomplete but not in any unsafe way), we should just not have it at all.
> You raise an interesting point about people who might be using 
> -frounding-math already. However, if they are using this flag because they 
> also sometimes build with a compiler such as gcc that supports the flag, they 
> are currently getting incorrect behavior from clang. Without this patch, 
> clang silently ignores the option and the optimizer silently ignores the fact 
> that the program may be changing the rounding mode dynamically. The user may 
> or may not be aware of this.
> 
> With this patch such a user is likely to observe two effects: (1) their code 
> will suddenly get slower, and (2) it will probably start behaving correctly 
> with regard to rounding mode changes. The rounding behavior will definitely 
> not get worse. I think the warning is useful as an indication that something 
> has changed. I don't think requiring an additional option should be necessary.
> However, if they are using this flag because they also sometimes build with a 
> compiler such as gcc that supports the flag, they are currently getting 
> incorrect behavior from clang

Incorrect, yes, but also likely valid behavior. "experimental" seems to imply a 
miscompile when using this option should not be unexpected.

As I suggested before: if I'm wrong, and this behavior is only going to make 
the code more correct (as you suggest), can we remove the warning that this 
must be ack'd explicitly by adding `-Wno-experimental-float-control` to builds? 
I don't understand the motivation for the warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



___
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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I don't insist on function redefinition solution. You want to replace functions 
- fine, but do this at the codegen, not in AST.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782670 , @hfinkel wrote:

> In D71241#1782652 , @ABataev wrote:
>
> > In D71241#1782648 , @hfinkel wrote:
> >
> > > In D71241#1782614 , @ABataev 
> > > wrote:
> > >
> > > > In D71241#1782551 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D71241#1779168 , 
> > > > > @JonChesterfield wrote:
> > > > >
> > > > > > Lowering in sema or in codegen seems a standard phase ordering 
> > > > > > choice. There will be pros and cons to both.
> > > > > >
> > > > > > I think prior art leans towards sema. Variants are loosely 
> > > > > > equivalent to tag dispatching or constexpr if, both handled before 
> > > > > > lowering the AST to IR.
> > > > >
> > > > >
> > > > > This is exactly right. This is just like any other kind of static 
> > > > > overload resolution. It should be resolved in Sema and the CallExpr's 
> > > > > DeclRefExpr should refer to the correct callee. This will make sure 
> > > > > that tools, including static analysis tools, will correctly 
> > > > > understand the semantics of the call (e.g., IPA will work correctly). 
> > > > > Also, we prefer, in Clang, to generate errors and warning messages in 
> > > > > Sema, not in CodeGen, and it is certainly plausible that errors and 
> > > > > warnings could be generated during the selector-based resolution 
> > > > > process.
> > > > >
> > > > > That having been said, Alexey is also correct that we retain the 
> > > > > unevaluated form of the constexpr expressions, and there is an 
> > > > > important analogy here. I believe that one way of restating Alexey's 
> > > > > concerns about the AST representation is that, if we resolve the 
> > > > > variant selection as we build the AST, and then we print the AST, the 
> > > > > printed function would be the name of the selected variant and not 
> > > > > the name of the base function. This is certainly a legitimate 
> > > > > concern, and there are several places in Clang where we take care to 
> > > > > preserve the spelling used for constructs that are otherwise 
> > > > > semantically equivalent (e.g., different spellings of keywords). I 
> > > > > can certainly imagine a tool wanting to see the base function called, 
> > > > > and we'll want that for the AST printer regardless. We might add this 
> > > > > information to CallExpr or make a new subclass of CallExpr (e.g., 
> > > > > OpenMPVariantCallExpr) that has that information (the latter seems 
> > > > > likely better so that we don't increase the size of CallExpr for an 
> > > > > uncommon case).
> > > > >
> > > > > > Writing the dispatch lowering on IR should make it easier to call 
> > > > > > from a Fortran front end. I'm in favour of minimising work done on 
> > > > > > the clang AST on general principles.
> > > > >
> > > > > We need to make the best decision for Clang in Clang, regardless of 
> > > > > how this might impact a future Fortran implementation. While the 
> > > > > OpenMPIRBuilder will be a point of reuse between different 
> > > > > OpenMP-enabled frontends, it need not be the only one. Moreover, 
> > > > > Fortran will also want to do this resolution earlier for the same 
> > > > > reason that it should be done earlier in Clang (and, for Fortran, 
> > > > > we'll end up with inlining and other IPA at the FIR level, so it will 
> > > > > be required to resolve the variants prior to hitting the 
> > > > > OpenMPIRBuilder). Thus, no, doing this in CodeGen is unlikely to work 
> > > > > for the Flang implementation.
> > > > >
> > > > > Also, "minimizing work done in the Clang AST on general principles", 
> > > > > seems like an oversimplification of our general Clang design 
> > > > > philosophy. Overload resolution in Clang is certainly a significant 
> > > > > part of the implementation, but we wouldn't consider doing it in 
> > > > > CodeGen. The AST should faithfully represent the semantic elements in 
> > > > > the source code. Overload resolution, template instantiation, 
> > > > > constexpr evaluation, etc. all are highly non-trivial, and all happen 
> > > > > during Sema (even in cases where we might, technically speaking, be 
> > > > > able to delay that logic until CodeGen). What we don't do in Sema are 
> > > > >  lowering tasks (e.g., translating references into pointers or other 
> > > > > things related to picking an underlying implementation strategy for 
> > > > > particular constructs) and optimizations - where we do them at all - 
> > > > > e.g., constant folding, some devirtualization, and so on are done in 
> > > > > CodeGen. For the most part, of course, we defer optimizations to 
> > > > > LLVM's optimizer.
> > > > >
> > > > > > Given we have two implementations, each at different points in the 
> > > > > > pipeline, it might be 

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782700 , @hfinkel wrote:

> In D71241#1782668 , @ABataev wrote:
>
> >
>
>
> ...
>
> >> While we talk a lot about what you think is bad about this solution it 
> >> seems we ignore the problems in the current one. Let me summarize a few:
> >> 
> >> - Take https://godbolt.org/z/XCjQUA where the wrong function is called in 
> >> the target region (because the "hack" to inject code in the wrong 
> >> definition is not applicable).
> > 
> > No time for it, just short answers. No definition for the variant - no 
> > definition for the base.
>
> Are the variants not permitted to be external functions?


Allowed, of course. But the alias/body will be emitted only if variant function 
is defined. Everyhing else is going to be resolved by the linker.


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] D71397: [clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/AST/ExternalASTSource.h:69
 
-  /// Whether this AST source also provides information for
-  /// semantic analysis.
-  bool SemaSource = false;
+  // LLVM-style RTTI.
+  static char ID;

aprantl wrote:
> /// LLVM-style RTTI.
It would be better if this had a dedicated type instead of using `char` and 
then `void*` as the parameter, just to prevent simple bugs where somebody 
passes in the wrong pointer.  Maybe you could have a non-copyable `ClassID` 
type and a `ClassRef` type with the sole constructor `ClassRef(const ClassID 
&)`?

The virtual method name is trying too hard to be succinct, and it doesn't 
matter because it's rarely used; I would recommend borrowing the name 
`isKindOf` from Objective-C.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71397



___
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#1782668 , @ABataev wrote:

>


...

>> While we talk a lot about what you think is bad about this solution it seems 
>> we ignore the problems in the current one. Let me summarize a few:
>> 
>> - Take https://godbolt.org/z/XCjQUA where the wrong function is called in 
>> the target region (because the "hack" to inject code in the wrong definition 
>> is not applicable).
> 
> No time for it, just short answers. No definition for the variant - no 
> definition for the base.

Are the variants not permitted to be external functions?


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] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGecaa9363303e: [clangd] Heuristically resolve dependent 
method calls (authored by nridge).

Changed prior to commit:
  https://reviews.llvm.org/D71240?vs=233681=233698#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,51 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
+  )cpp",
+
+  R"cpp(// FIXME: Heuristic resolution of dependent method
+// invoked via smart pointer
+template  struct S { void foo(); };
+template  struct unique_ptr {
+  T* operator->();
+};
+template 
+void test(unique_ptr>& V) {
+  V->fo^o();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +570,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](float);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +604,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -52,6 +52,40 @@
   return S;
 }
 
+// Given a dependent type and a member name, heuristically resolve the
+// name to one or more declarations.
+// The current heuristic is simply to look up the name in the primary
+// template. This is a heuristic because the template could potentially
+// have specializations that declare different members.
+// Multiple declarations could be returned if the name is overloaded
+// (e.g. an overloaded method in the primary template).
+// This heuristic will give the desired answer in many cases, e.g.
+// for a call to vector::size().
+std::vector
+getMembersReferencedViaDependentName(const Type *T, const DeclarationName ,
+ bool IsNonstaticMember) {
+  if (!T)
+return {};
+  if (auto *ICNT = T->getAs()) {
+T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
+  }
+  auto *TST = T->getAs();
+  if (!TST)
+return {};
+  const ClassTemplateDecl *TD = dyn_cast_or_null(
+  TST->getTemplateName().getAsTemplateDecl());
+  if (!TD)
+return {};
+  CXXRecordDecl *RD = TD->getTemplatedDecl();
+  if (!RD->hasDefinition())
+return {};
+  RD = RD->getDefinition();
+  return RD->lookupDependentName(Name, [=](const NamedDecl *D) {
+return IsNonstaticMember ? D->isCXXInstanceMember()
+ : !D->isCXXInstanceMember();
+  });
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and 

[PATCH] D71419: [clang] [test] Disable the test exhausting stack on NetBSD

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c6c1d0f4371: [clang] [test] Disable the test exhausting 
stack on NetBSD (authored by mgorny).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71419

Files:
  clang/test/SemaTemplate/instantiation-depth-default.cpp


Index: clang/test/SemaTemplate/instantiation-depth-default.cpp
===
--- clang/test/SemaTemplate/instantiation-depth-default.cpp
+++ clang/test/SemaTemplate/instantiation-depth-default.cpp
@@ -4,6 +4,9 @@
 // increases our per-frame stack usage enough that this test no longer fits
 // within our normal stack space allocation.
 // UNSUPPORTED: asan
+//
+// The default stack size on NetBSD is too small for this test.
+// UNSUPPORTED: system-netbsd
 
 template struct X : X {};
 // expected-error-re@8 {{recursive template instantiation exceeded maximum 
depth of 1024{{$


Index: clang/test/SemaTemplate/instantiation-depth-default.cpp
===
--- clang/test/SemaTemplate/instantiation-depth-default.cpp
+++ clang/test/SemaTemplate/instantiation-depth-default.cpp
@@ -4,6 +4,9 @@
 // increases our per-frame stack usage enough that this test no longer fits
 // within our normal stack space allocation.
 // UNSUPPORTED: asan
+//
+// The default stack size on NetBSD is too small for this test.
+// UNSUPPORTED: system-netbsd
 
 template struct X : X {};
 // expected-error-re@8 {{recursive template instantiation exceeded maximum depth of 1024{{$
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8e0bb3b2c24: [LTO] Support for embedding bitcode section 
during LTO (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68213

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Frontend/x86-embed-bitcode.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/test/LTO/X86/Inputs/start-lib1.ll
  llvm/test/LTO/X86/Inputs/start-lib2.ll
  llvm/test/LTO/X86/embed-bitcode.ll

Index: llvm/test/LTO/X86/embed-bitcode.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/embed-bitcode.ll
@@ -0,0 +1,28 @@
+; RUN: llvm-as %s -o %t1.o
+; RUN: llvm-as %p/Inputs/start-lib1.ll -o %t2.o
+; RUN: llvm-as %p/Inputs/start-lib2.ll -o %t3.o
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode=false -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; CHECK-ELF: .text
+; CHECK-ELF: .llvmbc
+
+; CHECK-LL: @_start
+; CHECK-LL: @foo
+; CHECK-LL: @bar
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_start() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib2.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib2.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib1.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib1.ll
@@ -0,0 +1,8 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
@@ -312,11 +313,30 @@
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
+static cl::opt EmbedBitcode(
+"lto-embed-bitcode", cl::init(false),
+cl::desc("Embed LLVM bitcode in object files produced by LTO"));
+
+static void EmitBitcodeSection(Module , Config ) {
+  if (!EmbedBitcode)
+return;
+  SmallVector Buffer;
+  raw_svector_ostream OS(Buffer);
+  WriteBitcodeToFile(M, OS);
+
+  std::unique_ptr Buf(
+  new SmallVectorMemoryBuffer(std::move(Buffer)));
+  llvm::EmbedBitcodeInModule(M, Buf->getMemBufferRef(), /*EmbedBitcode*/ true,
+ /*EmbedMarker*/ false, /*CmdArgs*/ nullptr);
+}
+
 void codegen(Config , TargetMachine *TM, AddStreamFn AddStream,
  unsigned Task, Module ) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
 return;
 
+  EmitBitcodeSection(Mod, Conf);
+
   std::unique_ptr DwoOut;
   SmallString<1024> DwoFile(Conf.SplitDwarfOutput);
   if (!Conf.DwoDir.empty()) {
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -24,9 +24,10 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Bitcode/BitcodeReader.h"
+#include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Bitstream/BitCodes.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
-#include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -4677,3 +4678,125 @@
 
   Out.write((char *)(), Buffer.size());
 }
+
+static const char *getSectionNameForBitcode(const Triple ) {
+  switch (T.getObjectFormat()) {
+  case Triple::MachO:
+return "__LLVM,__bitcode";
+  case Triple::COFF:
+  case 

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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1782652 , @ABataev wrote:

> In D71241#1782648 , @hfinkel wrote:
>
> > In D71241#1782614 , @ABataev wrote:
> >
> > > In D71241#1782551 , @hfinkel 
> > > wrote:
> > >
> > > > In D71241#1779168 , 
> > > > @JonChesterfield wrote:
> > > >
> > > > > Lowering in sema or in codegen seems a standard phase ordering 
> > > > > choice. There will be pros and cons to both.
> > > > >
> > > > > I think prior art leans towards sema. Variants are loosely equivalent 
> > > > > to tag dispatching or constexpr if, both handled before lowering the 
> > > > > AST to IR.
> > > >
> > > >
> > > > This is exactly right. This is just like any other kind of static 
> > > > overload resolution. It should be resolved in Sema and the CallExpr's 
> > > > DeclRefExpr should refer to the correct callee. This will make sure 
> > > > that tools, including static analysis tools, will correctly understand 
> > > > the semantics of the call (e.g., IPA will work correctly). Also, we 
> > > > prefer, in Clang, to generate errors and warning messages in Sema, not 
> > > > in CodeGen, and it is certainly plausible that errors and warnings 
> > > > could be generated during the selector-based resolution process.
> > > >
> > > > That having been said, Alexey is also correct that we retain the 
> > > > unevaluated form of the constexpr expressions, and there is an 
> > > > important analogy here. I believe that one way of restating Alexey's 
> > > > concerns about the AST representation is that, if we resolve the 
> > > > variant selection as we build the AST, and then we print the AST, the 
> > > > printed function would be the name of the selected variant and not the 
> > > > name of the base function. This is certainly a legitimate concern, and 
> > > > there are several places in Clang where we take care to preserve the 
> > > > spelling used for constructs that are otherwise semantically equivalent 
> > > > (e.g., different spellings of keywords). I can certainly imagine a tool 
> > > > wanting to see the base function called, and we'll want that for the 
> > > > AST printer regardless. We might add this information to CallExpr or 
> > > > make a new subclass of CallExpr (e.g., OpenMPVariantCallExpr) that has 
> > > > that information (the latter seems likely better so that we don't 
> > > > increase the size of CallExpr for an uncommon case).
> > > >
> > > > > Writing the dispatch lowering on IR should make it easier to call 
> > > > > from a Fortran front end. I'm in favour of minimising work done on 
> > > > > the clang AST on general principles.
> > > >
> > > > We need to make the best decision for Clang in Clang, regardless of how 
> > > > this might impact a future Fortran implementation. While the 
> > > > OpenMPIRBuilder will be a point of reuse between different 
> > > > OpenMP-enabled frontends, it need not be the only one. Moreover, 
> > > > Fortran will also want to do this resolution earlier for the same 
> > > > reason that it should be done earlier in Clang (and, for Fortran, we'll 
> > > > end up with inlining and other IPA at the FIR level, so it will be 
> > > > required to resolve the variants prior to hitting the OpenMPIRBuilder). 
> > > > Thus, no, doing this in CodeGen is unlikely to work for the Flang 
> > > > implementation.
> > > >
> > > > Also, "minimizing work done in the Clang AST on general principles", 
> > > > seems like an oversimplification of our general Clang design 
> > > > philosophy. Overload resolution in Clang is certainly a significant 
> > > > part of the implementation, but we wouldn't consider doing it in 
> > > > CodeGen. The AST should faithfully represent the semantic elements in 
> > > > the source code. Overload resolution, template instantiation, constexpr 
> > > > evaluation, etc. all are highly non-trivial, and all happen during Sema 
> > > > (even in cases where we might, technically speaking, be able to delay 
> > > > that logic until CodeGen). What we don't do in Sema are  lowering tasks 
> > > > (e.g., translating references into pointers or other things related to 
> > > > picking an underlying implementation strategy for particular 
> > > > constructs) and optimizations - where we do them at all - e.g., 
> > > > constant folding, some devirtualization, and so on are done in CodeGen. 
> > > > For the most part, of course, we defer optimizations to LLVM's 
> > > > optimizer.
> > > >
> > > > > Given we have two implementations, each at different points in the 
> > > > > pipeline, it might be constructive to each write down why you each 
> > > > > choose said point. I suspect the rationale is hidden by the 
> > > > > implementation details.
> > >
> > >
> > > Actually, early resolution will break tbe tools, not help them. It will 
> > > definitely break 

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782650 , @jdoerfert wrote:

> In D71241#1782614 , @ABataev wrote:
>
> > Actually, early resolution will break tbe tools, not help them. It will 
> > definitely break clangd, for example. The user will try to navigate to 
> > originally called function `base` and instead he will be redirected to the 
> > function `hst`.
>
>
> And that is a good thing. Even if you argue it is not, *only* in the Sema 
> solution the tools have *all* the information available to redirect to the 
> base or variant function.
>
> In D71241#1782612 , @ABataev wrote:
>
> > In D71241#1782504 , @jdoerfert 
> > wrote:
> >
> > > Here, both the original callee (`wrong_ast`) and the actual callee `cpu` 
> > > are shown at the call site.
> > >
> > > Why would we not want that?
> >
> >
> > You have wron idea about AST representation. If something is not printed in 
> > dump, it does not mean it does nit exist in AST.
>
>
> This is plain insulting (again) and beyond the point. I just shown you that 
> the proposed solution has *all* the information in the AST available to be 
> used by tools, codegen, ... I did so because you claimed that would not be 
> the case, e.g. the AST would not represent the program faithfully. As you 
> see, all the original information is available. However, you still refuse to 
> acknowledge that and instead try to discredit me. I am tired of this kind of 
> "discussion", we went down this road before and, as it was back then, there 
> is nothing to be gained. It is harmful for the community and it is insulting 
> towards me.
>
>  ---
>
> While we talk a lot about what you think is bad about this solution it seems 
> we ignore the problems in the current one. Let me summarize a few:
>
> - Take https://godbolt.org/z/XCjQUA where the wrong function is called in the 
> target region (because the "hack" to inject code in the wrong definition is 
> not applicable).


No time for it, just short answers. No definition for the variant - no 
definition for the base.

> - Take https://godbolt.org/z/Yi9Lht where the wrong function is called on the 
> host (no there is *no* alias hidden)

GlobalAlias can be emitted only for definitions. No definition for variant - no 
aliasing.

> - Take https://godbolt.org/z/2evvtN which shows that the alias solution is 
> incompatible with linking.

Undefined behavior according to the standard.

> - Take the `construct` context selector and the `begin/end declare variant` 
> construct which both cannot be implemented with aliases.




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] D70524: Support DebugInfo generation for auto return type for C++ functions.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70524#1779179 , @awpandey wrote:

> @dblaikie . I have removed the redundant test case. What else should I do in 
> this patch?


Please address this warning before committing:

  
/usr/local/google/home/blaikie/dev/llvm/src/clang/lib/CodeGen/CGDebugInfo.cpp:3100:3:
 warning: unannotated fall-through between switch labels 
[-Wimplicit-fallthrough]
case Type::Attributed:
^
  
/usr/local/google/home/blaikie/dev/llvm/src/clang/lib/CodeGen/CGDebugInfo.cpp:3100:3:
 note: insert 'break;' to avoid fall-through
case Type::Attributed:
^
break; 
  1 warning generated.

It looks like this implementation is a bit buggy in one way and incomplete in 
another:

1. even if the auto-returning function is defined, that function definition 
doesn't describe the concrete return type. Compare GCC and Clang's output for 
your example and note that... oh.

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?) GCC does use the unspecified type "auto" even back in DWARFv4 and it 
leaves the subprogram definition DIE without a DW_AT_type if the auto type ends 
up as void (what else could it do?) so I guess we can do this for consistency & 
consumers have to know that a definition can't really have an auto return type 
and that it must be really void.

In any case - change the test case to use a non-void return type in the 
definition ("return 3;" for instance, to get an int return type instead) and 
check that the DISubprogram for the definition has a concrete return type of 
"int" while the DISubprogram for the declaration has the "auto" 
unspecified_type return type. (contrast/test against GCC's behavior)

2. Presumably in a follow-up patch, make sure that the declaration for the 
DISubprogram declaration for an "auto" return type function appears in the 
member list of the DICompositeType even if the function is not called (same as 
other normal (non-implicit/non-template) functions) since that's the value of 
being able to describe the return type as "auto" (the function can be described 
even when the definition isn't available/emitted) - it doesn't currently. 
(contrast/test against with GCC's behavior)


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] D71431: Call objc_retainBlock before passing a block as a variadic argument

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Patch looks good, but could you describe this behavior in the ARC documentation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71431



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

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

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

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

- Fix some problems discovered during some real world stress test
- Add more tests
- The ASCII art is not updated yet, but will do so at some point.


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

https://reviews.llvm.org/D70470

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp

Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -0,0 +1,285 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+#define ZX_HANDLE_INVALID 0
+
+#if defined(__clang__)
+#define ZX_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define ZX_HANDLE_RELEASE  __attribute__((release_handle))
+#define ZX_HANDLE_USE  __attribute__((use_handle))
+#else
+#define ZX_HANDLE_ACQUIRE
+#define ZX_HANDLE_RELEASE
+#define ZX_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+zx_handle_t *out0 ZX_HANDLE_ACQUIRE,
+zx_handle_t *out1 ZX_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle ZX_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+void (*escape3)(zx_handle_t) = escape2; 
+
+void use1(const zx_handle_t *in ZX_HANDLE_USE);
+void use2(zx_handle_t in ZX_HANDLE_USE);
+
+void moreArgs(zx_handle_t, int, ...);
+void lessArgs(zx_handle_t, int a = 5);
+
+// To test if argument indexes are OK for operator calls.
+struct MyType {
+  ZX_HANDLE_ACQUIRE
+  zx_handle_t operator+(zx_handle_t ZX_HANDLE_RELEASE replace);
+};
+
+void checkInvalidHandle01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (sa == ZX_HANDLE_INVALID)
+;
+  // Will we ever see a warning like below?
+  // We eagerly replace the symbol with a constant and lose info...
+  use2(sa); // TODOexpected-warning {{Use of an invalid handle}}
+  zx_handle_close(sb);
+  zx_handle_close(sa);
+}
+
+void checkInvalidHandle2() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (sb != ZX_HANDLE_INVALID)
+zx_handle_close(sb);
+  if (sa != ZX_HANDLE_INVALID)
+zx_handle_close(sa);
+}
+
+void checkNoCrash01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  moreArgs(sa, 1, 2, 3, 4, 5);
+  lessArgs(sa);
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkLeak01(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  use1();
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag) {
+zx_handle_close(sa);
+use1(); // expected-warning {{Using a previously released handle}}
+  }
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
+
+void checkMemberOperatorIndices() {
+  zx_handle_t sa, sb, sc;
+  zx_channel_create(0, , );
+  zx_handle_close(sb);
+  MyType t;
+  sc = t + sa;
+  zx_handle_close(sc);
+}
+
+// RAII
+
+template 
+struct HandleWrapper {
+  ~HandleWrapper() { close(); }
+  void close() {
+if (handle != ZX_HANDLE_INVALID)
+  zx_handle_close(handle);
+  }
+  T *get_handle_address() { return  }
+private:
+  T handle;
+};
+
+void doNotWarnOnRAII() {
+  HandleWrapper w1;
+  zx_handle_t sb;
+  if 

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

[Zachary Henkel via Phabricator via cfe-commits]

zahen created this revision.
zahen added reviewers: rnk, thakis, hans, zturner.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

msvc allows a subsequent declaration of a uuid attribute on a struct/class.  
Mirror this behavior in clang-cl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71439

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/ms-uuid.cpp


Index: clang/test/SemaCXX/ms-uuid.cpp
===
--- clang/test/SemaCXX/ms-uuid.cpp
+++ clang/test/SemaCXX/ms-uuid.cpp
@@ -106,3 +106,9 @@
 }
 
 }
+
+// Test class/struct redeclaration where the subsequent
+// declaration has a uuid attribute
+struct X{};
+
+struct __declspec(uuid("----")) X;
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5394,9 +5394,11 @@
   if (const auto *UA = D->getAttr()) {
 if (UA->getGuid().equals_lower(Uuid))
   return nullptr;
-Diag(UA->getLocation(), diag::err_mismatched_uuid);
-Diag(CI.getLoc(), diag::note_previous_uuid);
-D->dropAttr();
+if (!UA->getGuid().empty()) {
+  Diag(UA->getLocation(), diag::err_mismatched_uuid);
+  Diag(CI.getLoc(), diag::note_previous_uuid);
+  D->dropAttr();
+}
   }
 
   return ::new (Context) UuidAttr(Context, CI, Uuid);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2678,6 +2678,10 @@
   // C's _Noreturn is allowed to be added to a function after it is 
defined.
   ++I;
   continue;
+} else if (isa(NewAttribute)) {
+  // msvc will allow a subsequent definition to add an uuid to a class
+  ++I;
+  continue;
 } else if (const AlignedAttr *AA = dyn_cast(NewAttribute)) {
   if (AA->isAlignas()) {
 // C++11 [dcl.align]p6:


Index: clang/test/SemaCXX/ms-uuid.cpp
===
--- clang/test/SemaCXX/ms-uuid.cpp
+++ clang/test/SemaCXX/ms-uuid.cpp
@@ -106,3 +106,9 @@
 }
 
 }
+
+// Test class/struct redeclaration where the subsequent
+// declaration has a uuid attribute
+struct X{};
+
+struct __declspec(uuid("----")) X;
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5394,9 +5394,11 @@
   if (const auto *UA = D->getAttr()) {
 if (UA->getGuid().equals_lower(Uuid))
   return nullptr;
-Diag(UA->getLocation(), diag::err_mismatched_uuid);
-Diag(CI.getLoc(), diag::note_previous_uuid);
-D->dropAttr();
+if (!UA->getGuid().empty()) {
+  Diag(UA->getLocation(), diag::err_mismatched_uuid);
+  Diag(CI.getLoc(), diag::note_previous_uuid);
+  D->dropAttr();
+}
   }
 
   return ::new (Context) UuidAttr(Context, CI, Uuid);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2678,6 +2678,10 @@
   // C's _Noreturn is allowed to be added to a function after it is defined.
   ++I;
   continue;
+} else if (isa(NewAttribute)) {
+  // msvc will allow a subsequent definition to add an uuid to a class
+  ++I;
+  continue;
 } else if (const AlignedAttr *AA = dyn_cast(NewAttribute)) {
   if (AA->isAlignas()) {
 // C++11 [dcl.align]p6:
___
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#1782430 , @JonChesterfield 
wrote:

> In D71241#1782427 , @ABataev wrote:
>
> > In D71241#1782425 , 
> > @JonChesterfield wrote:
> >
> > > > Explain that you're replacing the function written by the user on the 
> > > > fly by another one. If they accept it, go ahead.
> > >
> > > That's the observational effect of variants. Replacing is very similar to 
> > > calling + inlining.
> >
> >
> > Not in the AST.
>
>
> I don't see much difference between mutating the AST and mutating the SSA. 
> What're your objections to the former, specifically? It's not a stable 
> interface so tools hanging off it will have a process for updating when it 
> changes.


I'd like to add that what we're talking about is none of these things. We're 
not talking about "mutating" the AST at all. Neither are we inlining. We're 
talking about performing callee resolution when building the AST in the first 
place. This is exactly what we do in all other places where to do overload 
resolution.

This is different from other places where we perform overload resolution only 
in that the callee won't have the same name as the identifier used in the call 
expression. But that's okay - those are the semantics of the calls with OpenMP 
variants. You type one name, and the function that ends up being called has 
another name. But it's all static and part of the specified language semantics. 
Should we record the original "base" function? Yes. Should we represent it as 
the callee? No.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782648 , @hfinkel wrote:

> In D71241#1782614 , @ABataev wrote:
>
> > In D71241#1782551 , @hfinkel wrote:
> >
> > > In D71241#1779168 , 
> > > @JonChesterfield wrote:
> > >
> > > > Lowering in sema or in codegen seems a standard phase ordering choice. 
> > > > There will be pros and cons to both.
> > > >
> > > > I think prior art leans towards sema. Variants are loosely equivalent 
> > > > to tag dispatching or constexpr if, both handled before lowering the 
> > > > AST to IR.
> > >
> > >
> > > This is exactly right. This is just like any other kind of static 
> > > overload resolution. It should be resolved in Sema and the CallExpr's 
> > > DeclRefExpr should refer to the correct callee. This will make sure that 
> > > tools, including static analysis tools, will correctly understand the 
> > > semantics of the call (e.g., IPA will work correctly). Also, we prefer, 
> > > in Clang, to generate errors and warning messages in Sema, not in 
> > > CodeGen, and it is certainly plausible that errors and warnings could be 
> > > generated during the selector-based resolution process.
> > >
> > > That having been said, Alexey is also correct that we retain the 
> > > unevaluated form of the constexpr expressions, and there is an important 
> > > analogy here. I believe that one way of restating Alexey's concerns about 
> > > the AST representation is that, if we resolve the variant selection as we 
> > > build the AST, and then we print the AST, the printed function would be 
> > > the name of the selected variant and not the name of the base function. 
> > > This is certainly a legitimate concern, and there are several places in 
> > > Clang where we take care to preserve the spelling used for constructs 
> > > that are otherwise semantically equivalent (e.g., different spellings of 
> > > keywords). I can certainly imagine a tool wanting to see the base 
> > > function called, and we'll want that for the AST printer regardless. We 
> > > might add this information to CallExpr or make a new subclass of CallExpr 
> > > (e.g., OpenMPVariantCallExpr) that has that information (the latter seems 
> > > likely better so that we don't increase the size of CallExpr for an 
> > > uncommon case).
> > >
> > > > Writing the dispatch lowering on IR should make it easier to call from 
> > > > a Fortran front end. I'm in favour of minimising work done on the clang 
> > > > AST on general principles.
> > >
> > > We need to make the best decision for Clang in Clang, regardless of how 
> > > this might impact a future Fortran implementation. While the 
> > > OpenMPIRBuilder will be a point of reuse between different OpenMP-enabled 
> > > frontends, it need not be the only one. Moreover, Fortran will also want 
> > > to do this resolution earlier for the same reason that it should be done 
> > > earlier in Clang (and, for Fortran, we'll end up with inlining and other 
> > > IPA at the FIR level, so it will be required to resolve the variants 
> > > prior to hitting the OpenMPIRBuilder). Thus, no, doing this in CodeGen is 
> > > unlikely to work for the Flang implementation.
> > >
> > > Also, "minimizing work done in the Clang AST on general principles", 
> > > seems like an oversimplification of our general Clang design philosophy. 
> > > Overload resolution in Clang is certainly a significant part of the 
> > > implementation, but we wouldn't consider doing it in CodeGen. The AST 
> > > should faithfully represent the semantic elements in the source code. 
> > > Overload resolution, template instantiation, constexpr evaluation, etc. 
> > > all are highly non-trivial, and all happen during Sema (even in cases 
> > > where we might, technically speaking, be able to delay that logic until 
> > > CodeGen). What we don't do in Sema are  lowering tasks (e.g., translating 
> > > references into pointers or other things related to picking an underlying 
> > > implementation strategy for particular constructs) and optimizations - 
> > > where we do them at all - e.g., constant folding, some devirtualization, 
> > > and so on are done in CodeGen. For the most part, of course, we defer 
> > > optimizations to LLVM's optimizer.
> > >
> > > > Given we have two implementations, each at different points in the 
> > > > pipeline, it might be constructive to each write down why you each 
> > > > choose said point. I suspect the rationale is hidden by the 
> > > > implementation details.
> >
> >
> > Actually, early resolution will break tbe tools, not help them. It will 
> > definitely break clangd, for example. The user will try to navigate to 
> > originally called function `base` and instead he will be redirected to the 
> > function `hst`.
>
>
> Can you please be more specific? Please explain why the user would consider 
> this incorrect 

RE: [clang] 878a24e - Reapply "Fix crash on switch conditions of non-integer types in templates"

[Andrews, Elizabeth via cfe-commits]

Hi Alexander,

I am debugging this now but I assume the checks for the if condition were 
skipped before this commit (therefore no crash) because ‘c’ was set as type 
dependent. The checks are probably run now since c’s type dependency changed 
with this patch. The checks might need to be guarded better, but I am not sure. 
 I will take a look and get back to you ASAP.

Elizabeth

From: Alexander Kornienko 
Sent: Wednesday, December 11, 2019 2:35 PM
To: Andrews, Elizabeth ; Elizabeth Andrews 

Cc: cfe-commits 
Subject: Re: [clang] 878a24e - Reapply "Fix crash on switch conditions of 
non-integer types in templates"

After this commit clang started generating assertion failures on valid code. 
There are tons of instances in our codebase. Here's a reduced test case:
template
class a {
  int c : b;
  void f() {
if (c)
  ;
  }
};

Please take a look.

On Wed, Dec 4, 2019 at 12:39 AM Elizabeth Andrews via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:

Author: Elizabeth Andrews
Date: 2019-12-03T15:27:19-08:00
New Revision: 878a24ee244a24c39d1c57e9af2e88c621f7cce9

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

LOG: Reapply "Fix crash on switch conditions of non-integer types in templates"

This patch reapplies commit 759948467ea. Patch was reverted due to a
clang-tidy test fail on Windows. The test has been modified. There
are no additional code changes.

Patch was tested with ninja check-all on Windows and Linux.

Summary of code changes:

Clang currently crashes for switch statements inside a template when the
condition is a non-integer field member because contextual implicit
conversion is skipped when parsing the condition. This conversion is
however later checked in an assert when the case statement is handled.
The conversion is skipped when parsing the condition because
the field member is set as type-dependent based on its containing class.
This patch sets the type dependency based on the field's type instead.

This patch fixes Bug 40982.

Added:
clang/test/SemaTemplate/non-integral-switch-cond.cpp

Modified:

clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp
clang/test/SemaTemplate/dependent-names.cpp
clang/test/SemaTemplate/enum-argument.cpp
clang/test/SemaTemplate/member-access-expr.cpp

Removed:




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
index 18fe5ef4e5c2..2c288e0bbddf 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t
+// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t -- -- 
-fno-delayed-template-parsing

 namespace std {
 template
@@ -103,6 +103,8 @@ struct S {
   static constexpr T t = 0x8000;
   std::string s;
   void f(char c) { s += c | static_cast(t); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: an integer is interpreted as a 
chara
+  // CHECK-FIXES: {{^}}  void f(char c) { s += std::to_string(c | 
static_cast(t)); }
 };

 template S;

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
index 119eff67318e..8e546b44ab74 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -233,7 +233,7 @@ struct a {
 template 
 class d {
   a e;
-  void f() { e.b(); }
+  void f() { e.b(0); }
 };
 }  // namespace
 }  // namespace PR38055

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 322b3a7fa740..a73531ad5fad 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1678,6 +1678,15 @@ MemberExpr *MemberExpr::Create(
   MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
NameInfo, T, VK, OK, NOUR);

+  if (FieldDecl *Field = dyn_cast(MemberDecl)) {
+DeclContext *DC = MemberDecl->getDeclContext();
+// dyn_cast_or_null is used to handle objC variables which do not
+// have a declaration context.
+CXXRecordDecl *RD = dyn_cast_or_null(DC);
+if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC))
+  E->setTypeDependent(T->isDependentType());
+  }
+
   if (HasQualOrFound) {
 // FIXME: Wrong. We should be looking at the 

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

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71241#1782614 , @ABataev wrote:

> Actually, early resolution will break tbe tools, not help them. It will 
> definitely break clangd, for example. The user will try to navigate to 
> originally called function `base` and instead he will be redirected to the 
> function `hst`.


And that is a good thing. Even if you argue it is not, *only* in the Sema 
solution the tools have *all* the information available to redirect to the base 
or variant function.

In D71241#1782612 , @ABataev wrote:

> In D71241#1782504 , @jdoerfert wrote:
>
> > Here, both the original callee (`wrong_ast`) and the actual callee `cpu` 
> > are shown at the call site.
> >
> > Why would we not want that?
>
>
> You have wron idea about AST representation. If something is not printed in 
> dump, it does not mean it does nit exist in AST.


This is plain insulting (again) and beyond the point. I just shown you that the 
proposed solution has *all* the information in the AST available to be used by 
tools, codegen, ... I did so because you claimed that would not be the case, 
e.g. the AST would not represent the program faithfully. As you see, all the 
original information is available. However, you still refuse to acknowledge 
that and instead try to discredit me. I am tired of this kind of "discussion", 
we went down this road before and, as it was back then, there is nothing to be 
gained. It is harmful for the community and it is insulting towards me.

---

While we talk a lot about what you think is bad about this solution it seems we 
ignore the problems in the current one. Let me summarize a few:

- Take https://godbolt.org/z/XCjQUA where the wrong function is called in the 
target region (because the "hack" to inject code in the wrong definition is not 
applicable).
- Take https://godbolt.org/z/Yi9Lht where the wrong function is called on the 
host (no there is *no* alias hidden)
- Take https://godbolt.org/z/2evvtN which shows that the alias solution is 
incompatible with linking.
- Take the `construct` context selector and the `begin/end declare variant` 
construct which both cannot be implemented with aliases.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1778671 , @ABataev wrote:

> There can be another one issue with this solution with inline assembly. I’m 
> not completely sure about it, will try to investigate it tomorrow. I suggest 
> to discuss this solution with Richard Smith (or John McCall). If he/they are 
> ok with this transformation of the AST, we can switch to this scheme.


@jdoerfert , please do add Richard and John to this thread. We should be kind 
to them, however, and please write a summary of the language feature including 
some examples showing usage, and please also summarize the current 
implementation strategy and the one being proposed, so that they don't need to 
read the OpenMP spec to figure out what the discussion is about.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1782614 , @ABataev wrote:

> In D71241#1782551 , @hfinkel wrote:
>
> > In D71241#1779168 , 
> > @JonChesterfield wrote:
> >
> > > Lowering in sema or in codegen seems a standard phase ordering choice. 
> > > There will be pros and cons to both.
> > >
> > > I think prior art leans towards sema. Variants are loosely equivalent to 
> > > tag dispatching or constexpr if, both handled before lowering the AST to 
> > > IR.
> >
> >
> > This is exactly right. This is just like any other kind of static overload 
> > resolution. It should be resolved in Sema and the CallExpr's DeclRefExpr 
> > should refer to the correct callee. This will make sure that tools, 
> > including static analysis tools, will correctly understand the semantics of 
> > the call (e.g., IPA will work correctly). Also, we prefer, in Clang, to 
> > generate errors and warning messages in Sema, not in CodeGen, and it is 
> > certainly plausible that errors and warnings could be generated during the 
> > selector-based resolution process.
> >
> > That having been said, Alexey is also correct that we retain the 
> > unevaluated form of the constexpr expressions, and there is an important 
> > analogy here. I believe that one way of restating Alexey's concerns about 
> > the AST representation is that, if we resolve the variant selection as we 
> > build the AST, and then we print the AST, the printed function would be the 
> > name of the selected variant and not the name of the base function. This is 
> > certainly a legitimate concern, and there are several places in Clang where 
> > we take care to preserve the spelling used for constructs that are 
> > otherwise semantically equivalent (e.g., different spellings of keywords). 
> > I can certainly imagine a tool wanting to see the base function called, and 
> > we'll want that for the AST printer regardless. We might add this 
> > information to CallExpr or make a new subclass of CallExpr (e.g., 
> > OpenMPVariantCallExpr) that has that information (the latter seems likely 
> > better so that we don't increase the size of CallExpr for an uncommon case).
> >
> > > Writing the dispatch lowering on IR should make it easier to call from a 
> > > Fortran front end. I'm in favour of minimising work done on the clang AST 
> > > on general principles.
> >
> > We need to make the best decision for Clang in Clang, regardless of how 
> > this might impact a future Fortran implementation. While the 
> > OpenMPIRBuilder will be a point of reuse between different OpenMP-enabled 
> > frontends, it need not be the only one. Moreover, Fortran will also want to 
> > do this resolution earlier for the same reason that it should be done 
> > earlier in Clang (and, for Fortran, we'll end up with inlining and other 
> > IPA at the FIR level, so it will be required to resolve the variants prior 
> > to hitting the OpenMPIRBuilder). Thus, no, doing this in CodeGen is 
> > unlikely to work for the Flang implementation.
> >
> > Also, "minimizing work done in the Clang AST on general principles", seems 
> > like an oversimplification of our general Clang design philosophy. Overload 
> > resolution in Clang is certainly a significant part of the implementation, 
> > but we wouldn't consider doing it in CodeGen. The AST should faithfully 
> > represent the semantic elements in the source code. Overload resolution, 
> > template instantiation, constexpr evaluation, etc. all are highly 
> > non-trivial, and all happen during Sema (even in cases where we might, 
> > technically speaking, be able to delay that logic until CodeGen). What we 
> > don't do in Sema are  lowering tasks (e.g., translating references into 
> > pointers or other things related to picking an underlying implementation 
> > strategy for particular constructs) and optimizations - where we do them at 
> > all - e.g., constant folding, some devirtualization, and so on are done in 
> > CodeGen. For the most part, of course, we defer optimizations to LLVM's 
> > optimizer.
> >
> > > Given we have two implementations, each at different points in the 
> > > pipeline, it might be constructive to each write down why you each choose 
> > > said point. I suspect the rationale is hidden by the implementation 
> > > details.
>
>
> Actually, early resolution will break tbe tools, not help them. It will 
> definitely break clangd, for example. The user will try to navigate to 
> originally called function `base` and instead he will be redirected to the 
> function `hst`.


Can you please be more specific? Please explain why the user would consider 
this incorrect behavior. If the point of the tool is to allow the user to 
navigate to the function actually being called, then navigating to `base` seems 
incorrect if that's not the function being called. This is just like any other 
kind of overload resolution - 

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D71345#1781141 , @sammccall wrote:

> How do you feel about the approach here?


I agree that having more of the logic centralized (in `SelectionTree`) is 
preferable to having it directly in a call site like `getDeclAtPosition`.

I also agree that this makes the `SelectionTree` API a bit clunky.

I tested the behaviour in Eclipse CDT, on the following testcase:

  struct Material {
  friend Material operator+(Material, Material);
  };
  
  void foo() {
  Material hydrogen, oxygen;
  hydrogen^+^oxygen;
  }

Here, both navigations target the overloaded operator, but if you comment out 
the overloaded operator, they target the respective local variable 
declarations. I think this is pretty good do-what-I-mean behaviour, arguably 
better than a categorical preference for the right side over the left side.

I can describe how CDT achieves this:

- There is an API that takes a textual selection, and an AST node type, and 
finds the innermost node of that type that encloses the selection.
- The AST nodes built for the statement are as follows:

  hydrogen+oxygen;
  BCC;   // A and C have type Name, B has type ImplicitName



- (If the overloaded operator is commented out, there is no node B.)
- For the purpose of determining "encloses", the cursor being at either 
boundary of a node counts as a match (so A and B are both considered to enclose 
the first cursor, and B and C are both considered to enclose the second cursor).
- The algorithm for go-to-definition is: first look for an enclosing 
`ImplicitName` node; if one is found, go to its target. Otherwise, look for an 
enclosing `Name` node and go to its target.

Other features will use the same API, but perhaps with a different node type; 
for example, the "define out of line" refactoring will look for an enclosing 
`FunctionDeclarator` node.

Perhaps we could have a simpler `SelectionTree` API along these lines, where 
the type of desired node informs the selection at each call site?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D68520: [cmake] Fix clang builds with BUILD_SHARED=ON and CLANG_LINK_CLANG_DYLIB=ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D68520#1782387 , @tstellar wrote:

> After thinking about this more, do we want to even try to support the 
> BUILD_SHARED=ON + CLANG_LINK_CLANG_DYLIB=ON configuration?  We don't support 
> this in llvm.


Actually, even if we don't support this in clang, we still need to fix this.  
Otherwise libclang-cpp.so will not be useable at all when clang has been built 
with BUILD_SHARED=ON .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68520



___
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 233687.
sidneym added a comment.
Herald added a project: LLVM.

make check for lld more generic.


Repository:
  rL LLVM

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/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,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = 
Args.getLastArgValue(options::OPT_fuse_ld_EQ).contains_lower("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   
//
   // Silence warnings for various options
@@ -232,9 +234,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,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).contains_lower("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   //
   // Silence warnings for various options
@@ -232,9 +234,10 @@
   for (const auto  : HTC.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());
 
-  

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782551 , @hfinkel wrote:

> In D71241#1779168 , @JonChesterfield 
> wrote:
>
> > Lowering in sema or in codegen seems a standard phase ordering choice. 
> > There will be pros and cons to both.
> >
> > I think prior art leans towards sema. Variants are loosely equivalent to 
> > tag dispatching or constexpr if, both handled before lowering the AST to IR.
>
>
> This is exactly right. This is just like any other kind of static overload 
> resolution. It should be resolved in Sema and the CallExpr's DeclRefExpr 
> should refer to the correct callee. This will make sure that tools, including 
> static analysis tools, will correctly understand the semantics of the call 
> (e.g., IPA will work correctly). Also, we prefer, in Clang, to generate 
> errors and warning messages in Sema, not in CodeGen, and it is certainly 
> plausible that errors and warnings could be generated during the 
> selector-based resolution process.
>
> That having been said, Alexey is also correct that we retain the unevaluated 
> form of the constexpr expressions, and there is an important analogy here. I 
> believe that one way of restating Alexey's concerns about the AST 
> representation is that, if we resolve the variant selection as we build the 
> AST, and then we print the AST, the printed function would be the name of the 
> selected variant and not the name of the base function. This is certainly a 
> legitimate concern, and there are several places in Clang where we take care 
> to preserve the spelling used for constructs that are otherwise semantically 
> equivalent (e.g., different spellings of keywords). I can certainly imagine a 
> tool wanting to see the base function called, and we'll want that for the AST 
> printer regardless. We might add this information to CallExpr or make a new 
> subclass of CallExpr (e.g., OpenMPVariantCallExpr) that has that information 
> (the latter seems likely better so that we don't increase the size of 
> CallExpr for an uncommon case).
>
> > Writing the dispatch lowering on IR should make it easier to call from a 
> > Fortran front end. I'm in favour of minimising work done on the clang AST 
> > on general principles.
>
> We need to make the best decision for Clang in Clang, regardless of how this 
> might impact a future Fortran implementation. While the OpenMPIRBuilder will 
> be a point of reuse between different OpenMP-enabled frontends, it need not 
> be the only one. Moreover, Fortran will also want to do this resolution 
> earlier for the same reason that it should be done earlier in Clang (and, for 
> Fortran, we'll end up with inlining and other IPA at the FIR level, so it 
> will be required to resolve the variants prior to hitting the 
> OpenMPIRBuilder). Thus, no, doing this in CodeGen is unlikely to work for the 
> Flang implementation.
>
> Also, "minimizing work done in the Clang AST on general principles", seems 
> like an oversimplification of our general Clang design philosophy. Overload 
> resolution in Clang is certainly a significant part of the implementation, 
> but we wouldn't consider doing it in CodeGen. The AST should faithfully 
> represent the semantic elements in the source code. Overload resolution, 
> template instantiation, constexpr evaluation, etc. all are highly 
> non-trivial, and all happen during Sema (even in cases where we might, 
> technically speaking, be able to delay that logic until CodeGen). What we 
> don't do in Sema are  lowering tasks (e.g., translating references into 
> pointers or other things related to picking an underlying implementation 
> strategy for particular constructs) and optimizations - where we do them at 
> all - e.g., constant folding, some devirtualization, and so on are done in 
> CodeGen. For the most part, of course, we defer optimizations to LLVM's 
> optimizer.
>
> > Given we have two implementations, each at different points in the 
> > pipeline, it might be constructive to each write down why you each choose 
> > said point. I suspect the rationale is hidden by the implementation details.


Actually, early resolution will break tbe tools, not help them. It will 
definitely break clangd, for example. The user will try to navigate to 
originally called function `base` and instead he will be redirected to the 
function `hst`.


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

Re: [PATCH] D71419: [clang] [test] Disable the test exhausting stack on NetBSD

[Eric Christopher via cfe-commits]

As a quick note, this broke the test because you didn't update the line
numbers in the checks.

Fixed thusly:

echristo@athyra ~/r/llvm-project> git push
To github.com:llvm/llvm-project.git
   6bed43f3c40..259a9b10390  master -> master

:)

-eric

On Thu, Dec 12, 2019 at 12:25 PM Petr Hosek via Phabricator via cfe-commits
 wrote:

> phosek accepted this revision.
> phosek added a comment.
> This revision is now accepted and ready to land.
>
> LGTM
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D71419/new/
>
> https://reviews.llvm.org/D71419
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 259a9b1 - Update line number after previous patch added an additional unsupported

[Eric Christopher via cfe-commits]

Author: Eric Christopher
Date: 2019-12-12T14:45:05-08:00
New Revision: 259a9b1039093a1d036c1e6efcae699873f93ba5

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

LOG: Update line number after previous patch added an additional unsupported
and comment lines.

Added: 


Modified: 
clang/test/SemaTemplate/instantiation-depth-default.cpp

Removed: 




diff  --git a/clang/test/SemaTemplate/instantiation-depth-default.cpp 
b/clang/test/SemaTemplate/instantiation-depth-default.cpp
index 39410bd89b49..61619623c97c 100644
--- a/clang/test/SemaTemplate/instantiation-depth-default.cpp
+++ b/clang/test/SemaTemplate/instantiation-depth-default.cpp
@@ -9,10 +9,10 @@
 // UNSUPPORTED: system-netbsd
 
 template struct X : X {};
-// expected-error-re@8 {{recursive template instantiation exceeded maximum 
depth of 1024{{$
-// expected-note@8 {{instantiation of template class}}
-// expected-note@8 {{skipping 1023 contexts in backtrace}}
-// expected-note@8 {{use -ftemplate-depth=N to increase recursive template 
instantiation depth}}
+// expected-error-re@11 {{recursive template instantiation exceeded maximum 
depth of 1024{{$
+// expected-note@11 {{instantiation of template class}}
+// expected-note@11 {{skipping 1023 contexts in backtrace}}
+// expected-note@11 {{use -ftemplate-depth=N to increase recursive template 
instantiation depth}}
 
 X<0, int> x; // expected-note {{in instantiation of}}
 



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

[PATCH] D71434: [Driver] Use .init_array for all gcc installations and simplify Generic_ELF -fno-use-init-array rules

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think we should do this, but let's add a release note for it. There's a flag, 
so users always have a workaround if clang starts doing the wrong thing for 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71434



___
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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782504 , @jdoerfert wrote:

> In D71241#1782460 , @JonChesterfield 
> wrote:
>
> > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > > 
> > >   Faithfulness¶
> > >   The AST intends to provide a representation of the program that is 
> > > faithful to the original source. 
> >
> > That's pretty convincing.
>
>
> So let's actually look at the AST instead of just talking about it:
>
> We take the asm.cpp example from @ABataev that, as I argued earlier, shows 
> nicely why the alias solution does not work at all once we start thinking 
> about linking things.
>
> Now here is the code with calls to make it actually interesting:
>
>   static void cpu() { asm("nop"); }
>   
>#pragma omp declare variant(cpu) match(device = {kind(cpu)})
>static __attribute__((used)) void wrong_asm() {
>  asm ("xxx");
>}
>   
>void foo() {
>  cpu();
>  wrong_asm();
>}
>   
>
> In the current approach the as of foo looks like this:
>
>   `-FunctionDecl 0x563baf0af958  line:8:6 foo 'void ()'
> `-CompoundStmt 0x563baf0afb38 
>   |-CallExpr 0x563baf0afa78  'void'
>   | `-ImplicitCastExpr 0x563baf0afa60  'void (*)()' 
> 
>   |   `-DeclRefExpr 0x563baf0afa40  'void ()' lvalue Function 
> 0x563baf0af458 'cpu' 'void ()'
>   `-CallExpr 0x563baf0afb18  'void'
> `-ImplicitCastExpr 0x563baf0afb00  'void (*)()' 
> 
>   `-DeclRefExpr 0x563baf0afae0  'void ()' lvalue Function 
> 0x563baf0af668 'wrong_asm' 'void ()'
>
>
> As you might see, you don't see any hint of the declare variant stuff that 
> will eventually transform the `wrong_asm` call into a `cpu` call.
>
> In the proposed scheme, the AST looks like this:
>
>   `-FunctionDecl 0x1e53398  line:8:6 foo 'void ()'
> `-CompoundStmt 0x1e53580 
>   |-CallExpr 0x1e534b8  'void'
>   | `-ImplicitCastExpr 0x1e534a0  'void (*)()' 
> 
>   |   `-DeclRefExpr 0x1e53480  'void ()' lvalue Function 0x1e52e98 
> 'cpu' 'void ()'
>   `-CallExpr 0x1e53560  'void'
> `-ImplicitCastExpr 0x1e53548  'void (*)()' 
> 
>   `-DeclRefExpr 0x1e53520  'void ()' lvalue Function 0x1e52e98 
> 'cpu' 'void ()' (Function 0x1e530a8 'wrong_asm' 'void ()')
>
>
> Here, both the original callee (`wrong_ast`) and the actual callee `cpu` are 
> shown at the call site.
>
> Why would we not want that?


You have wron idea about AST representation. If something is not printed in 
dump, it does not mean it does nit exist in AST.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1779779 , @ABataev wrote:

> Here is the example that does not work with the proposed solution but works 
> with the existing one:
>
>   static void cpu() { asm("nop"); }
>  
>   #pragma omp declare variant(cpu) match(device = {kind(cpu)})
>   static __attribute__((used)) void wrong_asm() {
> asm ("xxx");
>   }
>
>
> The existing solution has some problems with the delayed error messages too, 
> but they are very easy to fix.


I don't understand that this example represents. Unused static functions are 
generally not emitted. In general, if we perform overload resolution in Sema 
and, thus, only use the variants that are selected, then others that are static 
won't even be emitted. Here you're forcing the `wrong_asm` function to be used, 
but if it's used on all devices (host and target), then it's used, and we need 
to deal with the inline asm regardless.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1782460 , @JonChesterfield 
wrote:

> > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > 
> >   Faithfulness¶
> >   The AST intends to provide a representation of the program that is 
> > faithful to the original source. 
>
> That's pretty convincing.


No, you're misinterpreting the intent of the statement. Here's the entire 
section...

> Faithfulness
>  The AST intends to provide a representation of the program that is faithful 
> to the original source. We intend for it to be possible to write refactoring 
> tools using only information stored in, or easily reconstructible from, the 
> Clang AST. This means that the AST representation should either not desugar 
> source-level constructs to simpler forms, or – where made necessary by 
> language semantics or a clear engineering tradeoff – should desugar minimally 
> and wrap the result in a construct representing the original source form.
> 
> For example, CXXForRangeStmt directly represents the syntactic form of a 
> range-based for statement, but also holds a semantic representation of the 
> range declaration and iterator declarations. It does not contain a 
> fully-desugared ForStmt, however.
> 
> Some AST nodes (for example, ParenExpr) represent only syntax, and others 
> (for example, ImplicitCastExpr) represent only semantics, but most nodes will 
> represent a combination of syntax and associated semantics. Inheritance is 
> typically used when representing different (but related) syntaxes for nodes 
> with the same or similar semantics.

First, being "faithful" to the original source means both syntax and semantics. 
I realize that AST is a somewhat-ambiguous term - we have semantic elements in 
our AST - but Clang's AST is not just some kind of minimized parse tree. The 
AST even has semantics-only nodes (e.g., for implicit casts). As you can see, 
the design considerations here are not just "record the local syntactic 
elements", but require semantic interpretation of these elements.

Again, Clang's AST is used for various kinds of static analysis tools, and 
these depend on having overload resolution correctly performed prior to that 
analysis. This includes overload resolution that depends on context (whether 
that's qualifications on `this` or host/device in CUDA or anything else).

None of this is to say that we should not record the original spelling of the 
function call, we should do that *also*, and that should be done when 
constructing the AST in Sema in addition to performing the variant selection.


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-tools-extra] ecaa936 - [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2019-12-12T17:18:00-05:00
New Revision: ecaa9363303e811a051ebb6199e35e43319a699c

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

LOG: [clangd] Heuristically resolve dependent method calls

Summary:
The heuristic is to look in the definition of the primary template,
which is what you want in the vast majority of cases.

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

Reviewers: sammccall

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

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 69c298b6887c..a4c17dbefded 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -52,6 +52,40 @@ nodeToString(const ast_type_traits::DynTypedNode ) {
   return S;
 }
 
+// Given a dependent type and a member name, heuristically resolve the
+// name to one or more declarations.
+// The current heuristic is simply to look up the name in the primary
+// template. This is a heuristic because the template could potentially
+// have specializations that declare 
diff erent members.
+// Multiple declarations could be returned if the name is overloaded
+// (e.g. an overloaded method in the primary template).
+// This heuristic will give the desired answer in many cases, e.g.
+// for a call to vector::size().
+std::vector
+getMembersReferencedViaDependentName(const Type *T, const DeclarationName 
,
+ bool IsNonstaticMember) {
+  if (!T)
+return {};
+  if (auto *ICNT = T->getAs()) {
+T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
+  }
+  auto *TST = T->getAs();
+  if (!TST)
+return {};
+  const ClassTemplateDecl *TD = dyn_cast_or_null(
+  TST->getTemplateName().getAsTemplateDecl());
+  if (!TD)
+return {};
+  CXXRecordDecl *RD = TD->getTemplatedDecl();
+  if (!RD->hasDefinition())
+return {};
+  RD = RD->getDefinition();
+  return RD->lookupDependentName(Name, [=](const NamedDecl *D) {
+return IsNonstaticMember ? D->isCXXInstanceMember()
+ : !D->isCXXInstanceMember();
+  });
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and (possibly) one type, but
@@ -79,9 +113,9 @@ nodeToString(const ast_type_traits::DynTypedNode ) {
 // formally size() is unresolved, but the primary template is a good guess.
 // This affects:
 //  - DependentTemplateSpecializationType,
-//  - DependentScopeMemberExpr
-//  - DependentScopeDeclRefExpr
 //  - DependentNameType
+//  - UnresolvedUsingValueDecl
+//  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = DeclRelationSet;
   using Rel = DeclRelation;
@@ -212,6 +246,32 @@ struct TargetFinder {
 break;
   }
   }
+  void
+  VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+const Type *BaseType = E->getBaseType().getTypePtrOrNull();
+if (E->isArrow()) {
+  // FIXME: Handle smart pointer types by looking up operator->
+  // in the primary template.
+  if (!BaseType || !BaseType->isPointerType()) {
+return;
+  }
+  BaseType = BaseType->getAs()
+ ->getPointeeType()
+ .getTypePtrOrNull();
+}
+for (const NamedDecl *D :
+ getMembersReferencedViaDependentName(BaseType, E->getMember(),
+  /*IsNonstaticMember=*/true)) 
{
+  Outer.add(D, Flags);
+}
+  }
+  void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {
+for (const NamedDecl *D : getMembersReferencedViaDependentName(
+ E->getQualifier()->getAsType(), E->getDeclName(),
+ /*IsNonstaticMember=*/false)) {
+  Outer.add(D, Flags);
+}
+  }
   void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *OIRE) {
 Outer.add(OIRE->getDecl(), Flags);
   }

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 058f205bb3a6..0352322790bd 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,51 @@ TEST(LocateSymbol, All) {
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method
+template 
+struct S {
+  

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:278
+});
+for (const NamedDecl *D : Decls) {
+  Outer.add(D, Flags);

sammccall wrote:
> sammccall wrote:
> > can we make this function return the decls, and make the callers call add() 
> > in a loop?
> > 
> > That way this function could be reused for the `operator->` lookup, and 
> > could be a standalone helper function outside this class. (Maybe eventually 
> > exposed in AST.h, as I can imagine it being useful in other places, but 
> > static in this file seems fine)
> with multiple lookup results we could add all, discard all, or pick one 
> arbitrarily.
> 
> I'm not sure what the best policy is, but it's worth pointing out in a 
> comment what the cases are and what the choice is.
> 
> I think this should only affects overloads, where returning all seems 
> reasonable (resolution would be way too much work).
> 
> Can you add a test?
Note, the additions to `LocateSymbol.Ambiguous` provide test coverage for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240



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

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 233681.
nridge marked an inline comment as done.
nridge added a comment.

Fix a typo and address one remaining comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,51 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
+  )cpp",
+
+  R"cpp(// FIXME: Heuristic resolution of dependent method
+// invoked via smart pointer
+template  struct S { void foo(); };
+template  struct unique_ptr {
+  T* operator->();
+};
+template 
+void test(unique_ptr>& V) {
+  V->fo^o();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +570,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](float);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +604,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -384,8 +384,8 @@
   // different kinds, deduplicate them.
   std::vector Result;
   for (const auto  : References) {
-if (auto Range = getTokenRange(AST.getSourceManager(),
-   AST.getLangOpts(), Ref.Loc)) {
+if (auto Range =
+getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
   DocumentHighlight DH;
   DH.range = *Range;
   if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -52,6 +52,40 @@
   return S;
 }
 
+// Given a dependent type and a member name, heuristically resolve the
+// name to one or more declarations.
+// The current heuristic is simply to look up the name in the primary
+// template. This is a heuristic because the template could potentially
+// have specializations that declare different members.
+// Multiple declarations could be returned if the name is overloaded
+// (e.g. an overloaded method in the primary template).
+// This heuristic will give the desired answer in many cases, e.g.
+// for a call to vector::size().
+std::vector
+getMembersReferencedViaDependentName(const Type *T, const DeclarationName ,
+ bool IsNonstaticMember) {
+  if (!T)
+return {};
+  if (auto *ICNT = T->getAs()) {
+T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
+  }
+  auto *TST = T->getAs();
+  if 

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 233680.
nridge marked 7 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,51 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of dependent static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
+  )cpp",
+
+  R"cpp(// FIXME: Heuristic resolution of dependent method
+// invoked via smart pointer
+template  struct S { void foo(); };
+template  struct unique_ptr {
+  T* operator->();
+};
+template 
+void test(unique_ptr>& V) {
+  V->fo^o();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +570,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](float);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +604,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -384,8 +384,8 @@
   // different kinds, deduplicate them.
   std::vector Result;
   for (const auto  : References) {
-if (auto Range = getTokenRange(AST.getSourceManager(),
-   AST.getLangOpts(), Ref.Loc)) {
+if (auto Range =
+getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
   DocumentHighlight DH;
   DH.range = *Range;
   if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -52,6 +52,40 @@
   return S;
 }
 
+// Given a dependent type and a member name, heuristically resolve the
+// name to one or more declarations.
+// The current heuristic is simply to look up the names in the primary
+// template. This is a heuristic because the template could potentially
+// have specializations that declare different members.
+// Multiple declarations could be returned if the name is overloaded
+// (e.g. an overloaded method in the primary template).
+// This heuristic will give the desired answer in many cases, e.g.
+// for a call to vector::size().
+std::vector
+getMembersReferencedViaDependentName(const Type *T, const DeclarationName ,
+ bool IsNonstaticMember) {
+  if (!T)
+return {};
+  if (auto *ICNT = T->getAs()) {
+T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
+  }
+  auto *TST = T->getAs();
+  if (!TST)
+return {};
+ 

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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1779168 , @JonChesterfield 
wrote:

> Lowering in sema or in codegen seems a standard phase ordering choice. There 
> will be pros and cons to both.
>
> I think prior art leans towards sema. Variants are loosely equivalent to tag 
> dispatching or constexpr if, both handled before lowering the AST to IR.


This is exactly right. This is just like any other kind of static overload 
resolution. It should be resolved in Sema and the CallExpr's DeclRefExpr should 
refer to the correct callee. This will make sure that tools, including static 
analysis tools, will correctly understand the semantics of the call (e.g., IPA 
will work correctly). Also, we prefer, in Clang, to generate errors and warning 
messages in Sema, not in CodeGen, and it is certainly plausible that errors and 
warnings could be generated during the selector-based resolution process.

That having been said, Alexey is also correct that we retain the unevaluated 
form of the constexpr expressions, and there is an important analogy here. I 
believe that one way of restating Alexey's concerns about the AST 
representation is that, if we resolve the variant selection as we build the 
AST, and then we print the AST, the printed function would be the name of the 
selected variant and not the name of the base function. This is certainly a 
legitimate concern, and there are several places in Clang where we take care to 
preserve the spelling used for constructs that are otherwise semantically 
equivalent (e.g., different spellings of keywords). I can certainly imagine a 
tool wanting to see the base function called, and we'll want that for the AST 
printer regardless. We might add this information to CallExpr or make a new 
subclass of CallExpr (e.g., OpenMPVariantCallExpr) that has that information 
(the latter seems likely better so that we don't increase the size of CallExpr 
for an uncommon case).

> Writing the dispatch lowering on IR should make it easier to call from a 
> Fortran front end. I'm in favour of minimising work done on the clang AST on 
> general principles.

We need to make the best decision for Clang in Clang, regardless of how this 
might impact a future Fortran implementation. While the OpenMPIRBuilder will be 
a point of reuse between different OpenMP-enabled frontends, it need not be the 
only one. Moreover, Fortran will also want to do this resolution earlier for 
the same reason that it should be done earlier in Clang (and, for Fortran, 
we'll end up with inlining and other IPA at the FIR level, so it will be 
required to resolve the variants prior to hitting the OpenMPIRBuilder). Thus, 
no, doing this in CodeGen is unlikely to work for the Flang implementation.

Also, "minimizing work done in the Clang AST on general principles", seems like 
an oversimplification of our general Clang design philosophy. Overload 
resolution in Clang is certainly a significant part of the implementation, but 
we wouldn't consider doing it in CodeGen. The AST should faithfully represent 
the semantic elements in the source code. Overload resolution, template 
instantiation, constexpr evaluation, etc. all are highly non-trivial, and all 
happen during Sema (even in cases where we might, technically speaking, be able 
to delay that logic until CodeGen). What we don't do in Sema are  lowering 
tasks (e.g., translating references into pointers or other things related to 
picking an underlying implementation strategy for particular constructs) and 
optimizations - where we do them at all - e.g., constant folding, some 
devirtualization, and so on are done in CodeGen. For the most part, of course, 
we defer optimizations to LLVM's optimizer.

> Given we have two implementations, each at different points in the pipeline, 
> it might be constructive to each write down why you each choose said point. I 
> suspect the rationale is hidden by the implementation details.


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] D71397: [clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

Side note: This https://reviews.llvm.org/D39111 seems related. The patch has 
languished as I have been busy with other work, but if it would be useful for 
you guys I'd be happy to try to land it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71397



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

[PATCH] D71428: [clang] Move CLANG_BOOTSTRAP_CMAKE_ARGS above PASSTHROUGH_VARIABLES

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added reviewers: beanz, phosek, compnerd, smeenai.
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71428



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

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

`unique_ptr` is harder to get to work than it might appear at first blush :)

Looking at the gcc 6 libstdc++ implementation as an example: the declared 
return type is not `T*`, but `unique_ptr::pointer`. The declaration of that 
is in turn:

  typedef typename _Pointer::type pointer;

The definition of `_Pointer` is in turn:

  // use SFINAE to determine whether _Del::pointer exists
  class _Pointer
  {
template
static typename _Up::pointer __test(typename _Up::pointer*);
  
template
static _Tp* __test(...);
  
typedef typename remove_reference<_Dp>::type _Del;
  
  public:
typedef decltype(__test<_Del>(0)) type;
  };

Writing heuristics that see through all that might be possible, and I'm happy 
to explore it, but definitely in a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240



___
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#1782460 , @JonChesterfield 
wrote:

> > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > 
> >   Faithfulness¶
> >   The AST intends to provide a representation of the program that is 
> > faithful to the original source. 
>
> That's pretty convincing.


So let's actually look at the AST instead of just talking about it:

We take the asm.cpp example from @ABataev that, as I argued earlier, shows 
nicely why the alias solution does not work at all once we start thinking about 
linking things.

Now here is the code with calls to make it actually interesting:

  static void cpu() { asm("nop"); }
  
  #pragma omp declare variant(cpu) match(device = {kind(cpu)})
  static __attribute__((used)) void wrong_asm() {
asm ("xxx");
  }
  
  void foo() {
cpu();
wrong_asm();
  }

In the current approach the as of foo looks like this:

  `-FunctionDecl 0x563baf0af958  line:8:6 foo 'void ()'
`-CompoundStmt 0x563baf0afb38 
  |-CallExpr 0x563baf0afa78  'void'
  | `-ImplicitCastExpr 0x563baf0afa60  'void (*)()' 

  |   `-DeclRefExpr 0x563baf0afa40  'void ()' lvalue Function 
0x563baf0af458 'cpu' 'void ()'
  `-CallExpr 0x563baf0afb18  'void'
`-ImplicitCastExpr 0x563baf0afb00  'void (*)()' 

  `-DeclRefExpr 0x563baf0afae0  'void ()' lvalue Function 
0x563baf0af668 'wrong_asm' 'void ()'

As you might see, you don't see any hint of the declare variant stuff that will 
eventually transform the `wrong_asm` call into a `cpu` call.

In the proposed scheme, the AST looks like this:

  `-FunctionDecl 0x1e53398  line:8:6 foo 'void ()'
`-CompoundStmt 0x1e53580 
  |-CallExpr 0x1e534b8  'void'
  | `-ImplicitCastExpr 0x1e534a0  'void (*)()' 

  |   `-DeclRefExpr 0x1e53480  'void ()' lvalue Function 0x1e52e98 
'cpu' 'void ()'
  `-CallExpr 0x1e53560  'void'
`-ImplicitCastExpr 0x1e53548  'void (*)()' 

  `-DeclRefExpr 0x1e53520  'void ()' lvalue Function 0x1e52e98 
'cpu' 'void ()' (Function 0x1e530a8 'wrong_asm' 'void ()')

Here, both the original callee (`wrong_ast`) and the actual callee `cpu` are 
shown at the call site.

Why would we not want that?


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] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
 
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+  "Support for floating point control option %0 is incomplete and 
experimental">,

rupprecht wrote:
> mibintc wrote:
> > @kpn thought it would be a good idea to add a Warning that the 
> > implementation of float control is experimental and partially implemented.  
> > That's what this is for.
> Instead of adding a warning, I'd like to propose `-frounding-math` not be 
> enabled unless an additional flag (e.g. `-fexperimental-float-control`) is 
> passed. Or maybe this feature should be called 
> `-f[no-]experimental-rounding-math` instead.
> 
> There are plenty of builds that are already specifying `-frounding-math` 
> (e.g. they also support building w/ a compiler such as gcc that implements 
> this), and adding this experimental/incomplete implementation is a surprise 
> to those builds.
> 
> If I'm wrong and it's completely safe to ignore the warning (maybe it's 
> incomplete but not in any unsafe way), we should just not have it at all.
You raise an interesting point about people who might be using -frounding-math 
already. However, if they are using this flag because they also sometimes build 
with a compiler such as gcc that supports the flag, they are currently getting 
incorrect behavior from clang. Without this patch, clang silently ignores the 
option and the optimizer silently ignores the fact that the program may be 
changing the rounding mode dynamically. The user may or may not be aware of 
this.

With this patch such a user is likely to observe two effects: (1) their code 
will suddenly get slower, and (2) it will probably start behaving correctly 
with regard to rounding mode changes. The rounding behavior will definitely not 
get worse. I think the warning is useful as an indication that something has 
changed. I don't think requiring an additional option should be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D71434: [Driver] Use .init_array for all gcc installations and simplify Generic_ELF -fno-use-init-array rules

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: echristo, rnk, thakis.
Herald added subscribers: cfe-commits, s.egerton, simoncook, fedor.sergeev, 
krytarowski, emaste, dschuff.
Herald added a project: clang.

D39317  made clang use .init_array when no gcc 
installations is found.
This change changes all gcc installations to use .init_array .

GCC 4.7 by default stopped providing .ctors/.dtors compatible crt files,
and stopped emitting .ctors for __attribute__((constructor)).
.init_array should always work.

FreeBSD rules are moved to FreeBSD.cpp to make Generic_ELF rules clean.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71434

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/constructors.c


Index: clang/test/Driver/constructors.c
===
--- clang/test/Driver/constructors.c
+++ clang/test/Driver/constructors.c
@@ -34,7 +34,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --gcc-toolchain="" \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -fuse-init-array \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2782,23 +2782,7 @@
 void Generic_ELF::addClangTargetOptions(const ArgList ,
 ArgStringList ,
 Action::OffloadKind) const {
-  const Generic_GCC::GCCVersion  = GCCInstallation.getVersion();
-  bool UseInitArrayDefault =
-  getTriple().getArch() == llvm::Triple::aarch64 ||
-  getTriple().getArch() == llvm::Triple::aarch64_be ||
-  (getTriple().isOSFreeBSD() &&
-   getTriple().getOSMajorVersion() >= 12) ||
-  (getTriple().getOS() == llvm::Triple::Linux &&
-   ((!GCCInstallation.isValid() || !V.isOlderThan(4, 7, 0)) ||
-getTriple().isAndroid())) ||
-  getTriple().getOS() == llvm::Triple::NaCl ||
-  (getTriple().getVendor() == llvm::Triple::MipsTechnologies &&
-   !getTriple().hasEnvironment()) ||
-  getTriple().getOS() == llvm::Triple::Solaris ||
-  getTriple().getArch() == llvm::Triple::riscv32 ||
-  getTriple().getArch() == llvm::Triple::riscv64;
-
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
-  options::OPT_fno_use_init_array, 
UseInitArrayDefault))
+  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fno-use-init-array");
 }
Index: clang/lib/Driver/ToolChains/FreeBSD.h
===
--- clang/lib/Driver/ToolChains/FreeBSD.h
+++ clang/lib/Driver/ToolChains/FreeBSD.h
@@ -76,6 +76,10 @@
   // Until dtrace (via CTF) and LLDB can deal with distributed debug info,
   // FreeBSD defaults to standalone/full debug info.
   bool GetDefaultStandaloneDebug() const override { return true; }
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
 
 protected:
   Tool *buildAssembler() const override;
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -467,3 +467,12 @@
 Res |= SanitizerKind::Memory;
   return Res;
 }
+
+void FreeBSD::addClangTargetOptions(const ArgList ,
+ArgStringList ,
+Action::OffloadKind) const {
+  if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
+  options::OPT_fno_use_init_array,
+  getTriple().getOSMajorVersion() >= 12))
+CC1Args.push_back("-fno-use-init-array");
+}


Index: clang/test/Driver/constructors.c
===
--- clang/test/Driver/constructors.c
+++ clang/test/Driver/constructors.c
@@ -34,7 +34,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --gcc-toolchain="" \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -fuse-init-array \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2782,23 +2782,7 @@
 void 

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

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 233675.
martong marked 4 inline comments as done.
martong added a comment.

Addressing Alexeis comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71020

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -247,6 +247,13 @@
   return cast(ET->getNamedType().getTypePtr())->getDecl();
 }
 
+static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
+  QualType Ty = FD->getFriendType()->getType().getCanonicalType();
+  if (isa(Ty))
+Ty = cast(Ty)->getNamedType();
+  return cast(Ty)->getDecl();
+}
+
 struct ImportExpr : TestImportBase {};
 struct ImportType : TestImportBase {};
 struct ImportDecl : TestImportBase {};
@@ -2707,7 +2714,7 @@
   EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
 }
 
-TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+TEST_P(ImportFriendFunctions, LookupWithProtoAfter) {
   auto FunctionPattern = functionDecl(hasName("f"));
   auto ClassPattern = cxxRecordDecl(hasName("X"));
 
@@ -3778,6 +3785,44 @@
   EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern));
 }
 
+TEST_P(ImportFriendClasses, UndeclaredFriendClassShouldNotBeVisible) {
+  Decl *FromTu = getTuDecl("class X { friend class Y; };", Lang_CXX, "from.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTu, cxxRecordDecl(hasName("X")));
+  auto *FromFriend = FirstDeclMatcher().match(FromTu, friendDecl());
+  RecordDecl *FromRecordOfFriend =
+  const_cast(getRecordDeclOfFriend(FromFriend));
+
+  ASSERT_EQ(FromRecordOfFriend->getDeclContext(), cast(FromTu));
+  ASSERT_EQ(FromRecordOfFriend->getLexicalDeclContext(),
+cast(FromX));
+  ASSERT_FALSE(
+  FromRecordOfFriend->getDeclContext()->containsDecl(FromRecordOfFriend));
+  ASSERT_FALSE(FromRecordOfFriend->getLexicalDeclContext()->containsDecl(
+  FromRecordOfFriend));
+  ASSERT_FALSE(FromRecordOfFriend->getLookupParent()
+   ->lookup(FromRecordOfFriend->getDeclName())
+   .empty());
+
+  auto *ToX = Import(FromX, Lang_CXX);
+  ASSERT_TRUE(ToX);
+
+  Decl *ToTu = ToX->getTranslationUnitDecl();
+  auto *ToFriend = FirstDeclMatcher().match(ToTu, friendDecl());
+  RecordDecl *ToRecordOfFriend =
+  const_cast(getRecordDeclOfFriend(ToFriend));
+
+  ASSERT_EQ(ToRecordOfFriend->getDeclContext(), cast(ToTu));
+  ASSERT_EQ(ToRecordOfFriend->getLexicalDeclContext(), cast(ToX));
+  EXPECT_FALSE(
+  ToRecordOfFriend->getDeclContext()->containsDecl(ToRecordOfFriend));
+  EXPECT_FALSE(ToRecordOfFriend->getLexicalDeclContext()->containsDecl(
+  ToRecordOfFriend));
+  EXPECT_FALSE(ToRecordOfFriend->getLookupParent()
+   ->lookup(ToRecordOfFriend->getDeclName())
+   .empty());
+}
+
 TEST_P(ImportFriendClasses, ImportOfRecursiveFriendClassTemplate) {
   Decl *FromTu = getTuDecl(
   R"(
@@ -4477,11 +4522,6 @@
   ASSERT_EQ(Res.size(), 0u);
 }
 
-static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
-  QualType Ty = FD->getFriendType()->getType().getCanonicalType();
-  return cast(Ty)->getDecl();
-}
-
 TEST_P(ASTImporterLookupTableTest,
LookupFindsFwdFriendClassDeclWithElaboratedType) {
   TranslationUnitDecl *ToTU = getToTuDecl(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -298,6 +298,45 @@
   return nullptr;
 }
 
+void addDeclToContexts(Decl *FromD, Decl *ToD) {
+  if (Importer.isMinimalImport()) {
+// In minimal import case the decl must be added even if it is not
+// contained in original context, for LLDB compatibility.
+// FIXME: Check if a better solution is possible.
+if (!FromD->getDescribedTemplate() &&
+FromD->getFriendObjectKind() == Decl::FOK_None)
+  ToD->getLexicalDeclContext()->addDeclInternal(ToD);
+return;
+  }
+
+  DeclContext *FromDC = FromD->getDeclContext();
+  DeclContext *FromLexicalDC = FromD->getLexicalDeclContext();
+  DeclContext *ToDC = ToD->getDeclContext();
+  DeclContext *ToLexicalDC = ToD->getLexicalDeclContext();
+
+  bool Visible = false;
+  if (FromDC->containsDeclAndLoad(FromD)) {
+ToDC->addDeclInternal(ToD);
+Visible = true;
+  }
+  if (ToDC != ToLexicalDC && FromLexicalDC->containsDeclAndLoad(FromD)) {
+ToLexicalDC->addDeclInternal(ToD);
+Visible = true;
+  }
+
+  // If the Decl was added to any context, it was made already visible.
+  // Otherwise it is still possible that it should be visible.
+  if (!Visible) {
+if (auto *FromNamed = 

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:334
+  FromDC->lookup(FromNamed->getDeclName());
+  if (std::find(FromLookup.begin(), FromLookup.end(), FromNamed) !=
+  FromLookup.end())

a_sidorin wrote:
> llvm::is_contained?
Thanks! Good catch.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:258
+static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
+  QualType Ty = FD->getFriendType()->getType();
+  if (auto *Inner = dyn_cast(Ty.getTypePtr())) {

a_sidorin wrote:
> Why don't we use getCanonicalType() anymore?
Thanks, that is a very good catch.


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] D71433: [analyzer] CERT: POS34-C

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze created this revision.
zukatsinadze added a reviewer: NoQ.
zukatsinadze added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny.

This patch introduces a new checker:
`alpha.security.cert.pos.34c`

This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument
The check warns if  `putenv ` function is
called with automatic storage variable as an argument.


Repository:
  rC Clang

https://reviews.llvm.org/D71433

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/cert/pos34-c.cpp

Index: clang/test/Analysis/cert/pos34-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/pos34-c.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.pos.34c\
+// RUN:  -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+void free(void *memblock);
+void *malloc(size_t size);
+int putenv(char *);
+int snprintf(char *str, size_t size, const char *format, ...);
+
+namespace test_auto_var_used_bad {
+
+// example from cert
+int volatile_memory1(const char *var) {
+  char env[1024];
+  int retval = snprintf(env, sizeof(env),"TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+ 
+  return putenv(env);
+  // expected-warning@-1 {{'putenv' function should not be called with auto variables}}
+}
+
+int volatile_memory2(char *a) {
+  return putenv(a);
+  // expected-warning@-1 {{'putenv' function should not be called with auto variables}}
+}
+
+void volatile_memory3(char *a) {
+  char *buff = (char *)"hello";
+  putenv(buff);
+  // expected-warning@-1 {{'putenv' function should not be called with auto variables}}
+}
+
+} // namespace test_auto_var_used_bad
+
+namespace test_auto_var_used_good {
+
+// example from cert
+int test_static(const char *var) {
+  static char env[1024];
+ 
+  int retval = snprintf(env, sizeof(env),"TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+ 
+  return putenv(env);
+}
+
+// example from cert
+int test_heap_memory(const char *var) {
+  static char *oldenv;
+  const char *env_format = "TEST=%s";
+  const size_t len = strlen(var) + strlen(env_format);
+  char *env = (char *)malloc(len);
+  if (env == NULL) {
+return -1;
+  }
+  if (putenv(env) != 0) { // no-warning: env was dynamically allocated.
+free(env);
+return -1;
+  }
+  if (oldenv != NULL) {
+free(oldenv); /* avoid memory leak */
+  }
+  oldenv = env;
+  return 0;
+}
+
+extern char *ex;
+int test_extern() {
+  return putenv(ex); // no-warning: extern storage class.
+}
+
+} // namespace test_auto_var_used_good
Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
===
--- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -9,12 +9,17 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 
 // Common strings used for the "category" of many static analyzer issues.
-namespace clang { namespace ento { namespace categories {
+namespace clang {
+namespace ento {
+namespace categories {
 
-const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
-const char * const LogicError = "Logic error";
-const char * const MemoryRefCount =
-  "Memory (Core Foundation/Objective-C/OSObject)";
-const char * const MemoryError = "Memory error";
-const char * const UnixAPI = "Unix API";
-}}}
+const char *const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
+const char *const LogicError = "Logic error";
+const char *const MemoryRefCount =
+"Memory (Core Foundation/Objective-C/OSObject)";
+const char *const MemoryError = "Memory error";
+const char *const UnixAPI = "Unix API";
+const char *const SecurityError = "Security error";
+} // namespace categories
+} // namespace ento
+} // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
@@ -0,0 +1,68 @@
+//== PutenvWithAutoChecker.cpp - -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: 

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

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

> https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> 
>   Faithfulness¶
>   The AST intends to provide a representation of the program that is faithful 
> to the original source. 

That's pretty convincing.


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] D71082: Allow system header to provide their own implementation of some builtin

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Just a few more nits, and this LGTM. Thanks again!

IDK if Eli wanted to have another look at this; please wait until tomorrow 
before landing to give him time to comment.




Comment at: clang/lib/AST/Decl.cpp:3006
 
+bool FunctionDecl::isInlineBuiltin() const {
+  if (!getBuiltinID())

nit: If we're going with this naming, I think it's important to highlight that 
we're querying if this is a definition. So `isInlineBuiltinDefinition()`?



Comment at: clang/test/CodeGen/memcpy-nobuiltin.c:5
+//
+// CHECK-WITH-DECL: 'memcpy' will always overflow; destination buffer has size 
1, but size argument is 2
+// CHECK-WITH-DECL-NOT: @llvm.memcpy

Generally, I think we try to keep diag checking and codegen checking separate, 
but this is simple enough that I don't think it's a big deal.

Please use clang's `-verify` + `expected-warning` instead of `CHECK`s here.

Looks like, among others, test/CodeGen/const-init.c combines `-verify` with 
FileCheck'ing codegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

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

> Are you going to add any tests in the following patches?

I don't have any tests for now, but in a later patch we can create them.




Comment at: clang/lib/AST/ASTImporter.cpp:8043
+
+  Error VisitFunctionTypeLoc(FunctionTypeLoc From) {
+auto To = ToL.castAs();

a_sidorin wrote:
> Does this import interacts well with type loc import partially done at L3258 
> (VisitFunctionDecl)? Should we merge them?
I think we should check here whether the given ParmVarDecl had been imported 
previously and if yes then set that with `setParam`, otherwise it could be set 
to a nullptr. @balazske what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018



___
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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782430 , @JonChesterfield 
wrote:

> In D71241#1782427 , @ABataev wrote:
>
> > In D71241#1782425 , 
> > @JonChesterfield wrote:
> >
> > > > Explain that you're replacing the function written by the user on the 
> > > > fly by another one. If they accept it, go ahead.
> > >
> > > That's the observational effect of variants. Replacing is very similar to 
> > > calling + inlining.
> >
> >
> > Not in the AST.
>
>
> I don't see much difference between mutating the AST and mutating the SSA. 
> What're your objections to the former, specifically? It's not a stable 
> interface so tools hanging off it will have a process for updating when it 
> changes.


https://clang.llvm.org/docs/InternalsManual.html#the-ast-library

  Faithfulness¶
  The AST intends to provide a representation of the program that is faithful 
to the original source. 


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D71241#1782427 , @ABataev wrote:

> In D71241#1782425 , @JonChesterfield 
> wrote:
>
> > > Explain that you're replacing the function written by the user on the fly 
> > > by another one. If they accept it, go ahead.
> >
> > That's the observational effect of variants. Replacing is very similar to 
> > calling + inlining.
>
>
> Not in the AST.


I don't see much difference between mutating the AST and mutating the SSA. 
What're your objections to the former, specifically? It's not a stable 
interface so tools hanging off it will have a process for updating when it 
changes.


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] 5ad5258 - [OPENMP50]Fix possible conflict when emitting an alias for the functions

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-12-12T15:48:33-05:00
New Revision: 5ad52587ec182f03636649e2cb66a0a4d9ffeab2

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

LOG: [OPENMP50]Fix possible conflict when emitting an alias for the functions
in declare variant.

If the types of the fnction are not equal, but match, at the codegen
thei may have different types. This may lead to compiler crash.

Added: 
clang/test/OpenMP/declare_variant_mixed_codegen.c

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index e3e9bd4284c3..1aae18b4504c 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1280,7 +1280,7 @@ bool CGOpenMPRuntime::tryEmitDeclareVariant(const 
GlobalDecl ,
   llvm::GlobalValue *Addr = CGM.GetGlobalValue(NewMangledName);
   if (Addr && !Addr->isDeclaration()) {
 const auto *D = cast(OldGD.getDecl());
-const CGFunctionInfo  = CGM.getTypes().arrangeGlobalDeclaration(OldGD);
+const CGFunctionInfo  = CGM.getTypes().arrangeGlobalDeclaration(NewGD);
 llvm::Type *DeclTy = CGM.getTypes().GetFunctionType(FI);
 
 // Create a reference to the named value.  This ensures that it is emitted

diff  --git a/clang/test/OpenMP/declare_variant_mixed_codegen.c 
b/clang/test/OpenMP/declare_variant_mixed_codegen.c
new file mode 100644
index ..63457095b934
--- /dev/null
+++ b/clang/test/OpenMP/declare_variant_mixed_codegen.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux 
-emit-llvm %s -o - | FileCheck %s --check-prefix HOST
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-linux -emit-pch -o %t 
-fopenmp-version=50 %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-linux -include-pch %t 
-verify %s -emit-llvm -o - -fopenmp-version=50 | FileCheck %s --check-prefix 
HOST
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc 
-fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple nvptx64-unknown-unknown 
-aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -fopenmp-version=50 | FileCheck 
%s --check-prefix GPU
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple nvptx64-unknown-unknown 
-aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t -fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple nvptx64-unknown-unknown 
-aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - 
-fopenmp-version=50 | FileCheck %s --check-prefix GPU
+// expected-no-diagnostics
+
+// HOST: @base = alias i32 (double), i32 (double)* @hst
+#ifndef HEADER
+#define HEADER
+
+int dev(double i) { return 0; }
+
+int hst(double i) { return 1; }
+
+#pragma omp declare variant(hst) match(device = {kind(host)})
+#pragma omp declare variant(dev) match(device = {kind(gpu)})
+int base();
+
+// HOST-LABEL: define void @foo()
+// HOST: call i32 (double, ...) bitcast (i32 (double)* @base to i32 (double, 
...)*)(double -1.00e+00)
+// HOST: call i32 @hst(double -2.00e+00)
+// HOST: call void [[OFFL:@.+_foo_l29]]()
+void foo() {
+  base(-1);
+  hst(-2);
+#pragma omp target
+  {
+base(-3);
+dev(-4);
+  }
+}
+
+// HOST: define {{.*}}void [[OFFL]]()
+// HOST: call i32 (double, ...) bitcast (i32 (double)* @base to i32 (double, 
...)*)(double -3.00e+00)
+// HOST: call i32 @dev(double -4.00e+00)
+
+// GPU: define {{.*}}void @__omp_offloading_{{.+}}_foo_l29()
+// GPU: call i32 @base(double -3.00e+00)
+// GPU: call i32 @dev(double -4.00e+00)
+
+// GPU: define {{.*}}i32 @base(double
+// GPU: ret i32 0
+// GPU: define {{.*}}i32 @dev(double
+// GPU: ret i32 0
+
+#endif // HEADER



___
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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782425 , @JonChesterfield 
wrote:

> > Explain that you're replacing the function written by the user on the fly 
> > by another one. If they accept it, go ahead.
>
> That's the observational effect of variants. Replacing is very similar to 
> calling + inlining.


Not in the AST.


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] D71241: [OpenMP][WIP] Use overload centric declare variants

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

> Explain that you're replacing the function written by the user on the fly by 
> another one. If they accept it, go ahead.

That's the observational effect of variants. Replacing is very similar to 
calling + inlining.


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] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
 
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+  "Support for floating point control option %0 is incomplete and 
experimental">,

mibintc wrote:
> @kpn thought it would be a good idea to add a Warning that the implementation 
> of float control is experimental and partially implemented.  That's what this 
> is for.
Instead of adding a warning, I'd like to propose `-frounding-math` not be 
enabled unless an additional flag (e.g. `-fexperimental-float-control`) is 
passed. Or maybe this feature should be called 
`-f[no-]experimental-rounding-math` instead.

There are plenty of builds that are already specifying `-frounding-math` (e.g. 
they also support building w/ a compiler such as gcc that implements this), and 
adding this experimental/incomplete implementation is a surprise to those 
builds.

If I'm wrong and it's completely safe to ignore the warning (maybe it's 
incomplete but not in any unsafe way), we should just not have it at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, arphaman.
ahatanak added a project: clang.
Herald added subscribers: ributzka, dexonsmith, jkorous.

Copy the block to the heap before passing it to the callee in case the block 
escapes in the callee.

rdar://problem/55683462


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71431

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenObjC/arc-blocks.m


Index: clang/test/CodeGenObjC/arc-blocks.m
===
--- clang/test/CodeGenObjC/arc-blocks.m
+++ clang/test/CodeGenObjC/arc-blocks.m
@@ -730,5 +730,15 @@
   test20_callee(^{ (void)x; });
 }
 
+// CHECK-LABEL: define void @test21(
+// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock(
+// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()*
+// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]),
+
+void test21_callee(int n, ...);
+void test21(id x) {
+  test21_callee(1, ^{ (void)x; });
+}
+
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK-UNOPT: attributes [[NUW]] = { nounwind }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5247,6 +5247,9 @@
   for (Expr *A : Args.slice(ArgIx)) {
 ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl);
 Invalid |= Arg.isInvalid();
+// Copy blocks to the heap.
+if (A->getType()->isBlockPointerType())
+  maybeExtendBlockObject(Arg);
 AllArgs.push_back(Arg.get());
   }
 }


Index: clang/test/CodeGenObjC/arc-blocks.m
===
--- clang/test/CodeGenObjC/arc-blocks.m
+++ clang/test/CodeGenObjC/arc-blocks.m
@@ -730,5 +730,15 @@
   test20_callee(^{ (void)x; });
 }
 
+// CHECK-LABEL: define void @test21(
+// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock(
+// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()*
+// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]),
+
+void test21_callee(int n, ...);
+void test21(id x) {
+  test21_callee(1, ^{ (void)x; });
+}
+
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK-UNOPT: attributes [[NUW]] = { nounwind }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5247,6 +5247,9 @@
   for (Expr *A : Args.slice(ArgIx)) {
 ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl);
 Invalid |= Arg.isInvalid();
+// Copy blocks to the heap.
+if (A->getType()->isBlockPointerType())
+  maybeExtendBlockObject(Arg);
 AllArgs.push_back(Arg.get());
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c8e0bb3 - [LTO] Support for embedding bitcode section during LTO

[Teresa Johnson via cfe-commits]

Author: Teresa Johnson
Date: 2019-12-12T12:34:19-08:00
New Revision: c8e0bb3b2c24ef59556d81a275fb1f5db64899d3

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

LOG: [LTO] Support for embedding bitcode section during LTO

Summary:
This adds support for embedding bitcode in a binary during LTO. The libLTO 
gains supports the `-lto-embed-bitcode` flag. The option allows users of the 
LTO library to embed a bitcode section. For example, LLD can pass the option 
via `ld.lld -mllvm=-lto-embed-bitcode`.

This feature allows doing something comparable to `clang -c -fembed-bitcode`, 
but on the (LTO) linker level. Having bitcode alongside native code has many 
use-cases. To give an example, the MacOS linker can create a `-bitcode_bundle` 
section containing bitcode. Also, having this feature built into LLVM is an 
alternative to 3rd party tools such as [[ 
https://github.com/travitch/whole-program-llvm | wllvm ]] or [[ 
https://github.com/SRI-CSL/gllvm | gllvm ]]. As with these tools, this feature 
simplifies creating "whole-program" llvm bitcode files, but in contrast to 
wllvm/gllvm it does not rely on a specific llvm frontend/driver.

Patch by Josef Eisl 

Reviewers: #llvm, #clang, rsmith, pcc, alexshap, tejohnson

Reviewed By: tejohnson

Subscribers: tejohnson, mehdi_amini, inglorion, hiraditya, aheejin, steven_wu, 
dexonsmith, dang, cfe-commits, llvm-commits, #llvm, #clang

Tags: #clang, #llvm

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

Added: 
clang/test/Frontend/x86-embed-bitcode.ll
llvm/test/LTO/X86/Inputs/start-lib1.ll
llvm/test/LTO/X86/Inputs/start-lib2.ll
llvm/test/LTO/X86/embed-bitcode.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Bitcode/BitcodeWriter.h
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/LTO/LTOBackend.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index c54d15dcdd58..7cf6041bea91 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1558,129 +1558,14 @@ void clang::EmitBackendOutput(DiagnosticsEngine ,
   }
 }
 
-static const char* getSectionNameForBitcode(const Triple ) {
-  switch (T.getObjectFormat()) {
-  case Triple::MachO:
-return "__LLVM,__bitcode";
-  case Triple::COFF:
-  case Triple::ELF:
-  case Triple::Wasm:
-  case Triple::UnknownObjectFormat:
-return ".llvmbc";
-  case Triple::XCOFF:
-llvm_unreachable("XCOFF is not yet implemented");
-break;
-  }
-  llvm_unreachable("Unimplemented ObjectFormatType");
-}
-
-static const char* getSectionNameForCommandline(const Triple ) {
-  switch (T.getObjectFormat()) {
-  case Triple::MachO:
-return "__LLVM,__cmdline";
-  case Triple::COFF:
-  case Triple::ELF:
-  case Triple::Wasm:
-  case Triple::UnknownObjectFormat:
-return ".llvmcmd";
-  case Triple::XCOFF:
-llvm_unreachable("XCOFF is not yet implemented");
-break;
-  }
-  llvm_unreachable("Unimplemented ObjectFormatType");
-}
-
 // With -fembed-bitcode, save a copy of the llvm IR as data in the
 // __LLVM,__bitcode section.
 void clang::EmbedBitcode(llvm::Module *M, const CodeGenOptions ,
  llvm::MemoryBufferRef Buf) {
   if (CGOpts.getEmbedBitcode() == CodeGenOptions::Embed_Off)
 return;
-
-  // Save llvm.compiler.used and remote it.
-  SmallVector UsedArray;
-  SmallPtrSet UsedGlobals;
-  Type *UsedElementType = Type::getInt8Ty(M->getContext())->getPointerTo(0);
-  GlobalVariable *Used = collectUsedGlobalVariables(*M, UsedGlobals, true);
-  for (auto *GV : UsedGlobals) {
-if (GV->getName() != "llvm.embedded.module" &&
-GV->getName() != "llvm.cmdline")
-  UsedArray.push_back(
-  ConstantExpr::getPointerBitCastOrAddrSpaceCast(GV, UsedElementType));
-  }
-  if (Used)
-Used->eraseFromParent();
-
-  // Embed the bitcode for the llvm module.
-  std::string Data;
-  ArrayRef ModuleData;
-  Triple T(M->getTargetTriple());
-  // Create a constant that contains the bitcode.
-  // In case of embedding a marker, ignore the input Buf and use the empty
-  // ArrayRef. It is also legal to create a bitcode marker even Buf is empty.
-  if (CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Marker) {
-if (!isBitcode((const unsigned char *)Buf.getBufferStart(),
-   (const unsigned char *)Buf.getBufferEnd())) {
-  // If the input is LLVM Assembly, bitcode is produced by serializing
-  // the module. Use-lists order need to be perserved in this case.
-  llvm::raw_string_ostream OS(Data);
-  llvm::WriteBitcodeToFile(*M, OS, /* ShouldPreserveUseListOrder */ true);
-  ModuleData =
-  ArrayRef((const uint8_t *)OS.str().data(), OS.str().size());
-} else
-  // If the input is LLVM 

[clang] 0ee89c1 - [OPENMP50]Improve checks for declare variant functions compatibility.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-12-12T15:18:19-05:00
New Revision: 0ee89c1bad8cef81725ef892d60b4aa254d84744

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

LOG: [OPENMP50]Improve checks for declare variant functions compatibility.

Added check for functions compatibility in C and removed restriction
for functions with no prototypes in declare variant constrcut.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_variant_messages.c
clang/test/OpenMP/declare_variant_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 423ccbd293d2..c91ef356f944 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9696,10 +9696,6 @@ def warn_omp_declare_variant_after_used : Warning<
 def warn_omp_declare_variant_after_emitted : Warning<
   "'#pragma omp declare variant' cannot be applied to the function that was 
defined already;"
   " the original function might be used">, InGroup;
-def err_omp_declare_variant_noproto : Error<
-  "function with '#pragma omp declare variant' must have a prototype">;
-def note_omp_declare_variant_specified_here : Note<
-  "'#pragma omp declare variant' for function specified here">;
 def err_omp_declare_variant_doesnt_support : Error<
   "'#pragma omp declare variant' does not "
   "support %select{function templates|virtual functions|"

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c25712452013..ebc919b52a49 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9828,13 +9828,15 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
 Linkage = 5,
   };
 
-  if (OldFD && !OldFD->getType()->getAs()) {
+  if (NoProtoDiagID.getDiagID() != 0 && OldFD &&
+  !OldFD->getType()->getAs()) {
 Diag(OldFD->getLocation(), NoProtoDiagID);
 Diag(NoteCausedDiagIDAt.first, NoteCausedDiagIDAt.second);
 return true;
   }
 
-  if (!NewFD->getType()->getAs())
+  if (NoProtoDiagID.getDiagID() != 0 &&
+  !NewFD->getType()->getAs())
 return Diag(NewFD->getLocation(), NoProtoDiagID);
 
   if (!TemplatesSupported &&

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index d876f04192b0..e02c1c591dff 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -23,6 +23,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/OpenMPKinds.h"
+#include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
@@ -5183,6 +5184,29 @@ Sema::DeclGroupPtrTy 
Sema::ActOnOpenMPDeclareSimdDirective(
   return DG;
 }
 
+static void setPrototype(Sema , FunctionDecl *FD, FunctionDecl *FDWithProto,
+ QualType NewType) {
+  assert(NewType->isFunctionProtoType() &&
+ "Expected function type with prototype.");
+  assert(FD->getType()->isFunctionNoProtoType() &&
+ "Expected function with type with no prototype.");
+  assert(FDWithProto->getType()->isFunctionProtoType() &&
+ "Expected function with prototype.");
+  // Synthesize parameters with the same types.
+  FD->setType(NewType);
+  SmallVector Params;
+  for (const ParmVarDecl *P : FDWithProto->parameters()) {
+auto *Param = ParmVarDecl::Create(S.getASTContext(), FD, SourceLocation(),
+  SourceLocation(), nullptr, P->getType(),
+  /*TInfo=*/nullptr, SC_None, nullptr);
+Param->setScopeInfo(0, Params.size());
+Param->setImplicit();
+Params.push_back(Param);
+  }
+
+  FD->setParams(Params);
+}
+
 Optional>
 Sema::checkOpenMPDeclareVariantFunction(Sema::DeclGroupPtrTy DG,
 Expr *VariantRef, SourceRange SR) {
@@ -5281,7 +5305,9 @@ 
Sema::checkOpenMPDeclareVariantFunction(Sema::DeclGroupPtrTy DG,
 if (ICS.isFailure()) {
   Diag(VariantRef->getExprLoc(),
diag::err_omp_declare_variant_incompat_types)
-  << VariantRef->getType() << FnPtrType << 
VariantRef->getSourceRange();
+  << VariantRef->getType()
+  << ((Method && !Method->isStatic()) ? FnPtrType : FD->getType())
+  << VariantRef->getSourceRange();
   return None;
 }
 VariantRefCast = PerformImplicitConversion(
@@ -5321,6 +5347,24 @@ 
Sema::checkOpenMPDeclareVariantFunction(Sema::DeclGroupPtrTy DG,
 return None;
   }
 
+  // Check if function types are compatible in C.
+  if (!LangOpts.CPlusPlus) {
+QualType NewType =
+