RE: Status of CET support? (Re: [PATCH] D40224:...)

[Ben Simhon, Oren via cfe-commits]

Hi Kostya,

Sorry for the detailed response.
I forwarded your questions to GCC team who are also adding CET support for 
Glibc / Linux Kernel / Loader.

I also talked to the simulations team to understand their timeline.
They will contact this mailing list with detailed answers.

There are few more patches to complete LLVM Compiler support.
I will appreciate your help in reviewing some of them:
https://reviews.llvm.org/D40482
https://reviews.llvm.org/D40478

Thanks,
Oren

From: Kostya Serebryany [mailto:k...@google.com]
Sent: Tuesday, November 28, 2017 01:46
Cc: Ben Simhon, Oren ; pavel.v.chu...@gmail.com; 
Keane, Erich ; Justin Bogner ; 
ablik...@gmail.com; Reid Kleckner ; Craig Topper 
; cfe-commits ; Evgeniy 
Stepanov ; Peter Collingbourne ; Sahita, 
Ravi ; Zuckerman, Michael 
Subject: Status of CET support? (Re: [PATCH] D40224:...)

Hi,

I see these patches for the CET support in LLVM -- great!
Could you please also tell us
  * what's the status of the Linux Kernel and Glibc support for CET?
  * how we can start evaluating the hardware feature (simulators, etc)?

Thanks!

--kcc


On Sun, Nov 26, 2017 at 4:35 AM, Phabricator via Phabricator via cfe-commits 
> wrote:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318995: Control-Flow Enforcement Technology - Shadow Stack 
and Indirect Branch Tracking… (authored by orenb).

Changed prior to commit:
  https://reviews.llvm.org/D40224?vs=123937=124287#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40224

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/lib/Basic/Targets/X86.h
  cfe/trunk/lib/Headers/CMakeLists.txt
  cfe/trunk/lib/Headers/cetintrin.h
  cfe/trunk/lib/Headers/immintrin.h
  cfe/trunk/test/CodeGen/builtins-x86.c
  cfe/trunk/test/CodeGen/cetintrin.c
  cfe/trunk/test/Driver/x86-target-features.c
  cfe/trunk/test/Preprocessor/x86_target_features.c


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

-
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via cfe-commits]

Author: martell
Date: Tue Nov 28 23:25:12 2017
New Revision: 319297

URL: http://llvm.org/viewvc/llvm-project?rev=319297=rev
Log:
Toolchain: Normalize dwarf, sjlj and seh eh

This is a re-apply of r319294.

adds -fseh-exceptions and -fdwarf-exceptions flags

clang will check if the user has specified an exception model flag,
in the absense of specifying the exception model clang will then check
the driver default and append the model flag for that target to cc1

-fno-exceptions has a higher priority then specifying the model

move __SEH__ macro definitions out of Targets into InitPreprocessor
behind the -fseh-exceptions flag

move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check

remove unused USESEHExceptions from the MinGW Driver

fold USESjLjExceptions into a new GetExceptionModel function that
gives the toolchain classes more flexibility with eh models

Reviewers: rnk, mstorsjo

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

Added:
cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.h
cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.h
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
cfe/trunk/test/Preprocessor/arm-target-features.c
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=319297=319296=319297=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 23:25:12 2017
@@ -1706,10 +1706,18 @@ Which overload candidates to show when o
 
 Enable C++14 sized global deallocation functions
 
+.. option:: -fdwarf-exceptions
+
+Use DWARF style exceptions
+
 .. option:: -fsjlj-exceptions
 
 Use SjLj style exceptions
 
+.. option:: -fseh-exceptions
+
+Use SEH style exceptions
+
 .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
 
 Enable the superword-level parallelism vectorization passes

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=319297=319296=319297=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Tue Nov 28 23:25:12 2017
@@ -124,7 +124,9 @@ LANGOPT(ZVector   , 1, 0, "Syste
 LANGOPT(Exceptions, 1, 0, "exception handling")
 LANGOPT(ObjCExceptions, 1, 0, "Objective-C exceptions")
 LANGOPT(CXXExceptions , 1, 0, "C++ exceptions")
+LANGOPT(DWARFExceptions   , 1, 0, "dwarf exception handling")
 LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling")
+LANGOPT(SEHExceptions , 1, 0, "SEH .xdata exception handling")
 LANGOPT(ExternCNoUnwind   , 1, 0, "Assume extern C functions don't unwind")
 LANGOPT(TraditionalCPP, 1, 0, "traditional CPP emulation")
 LANGOPT(RTTI  , 1, 1, "run-time type information")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=319297=319296=319297=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Nov 28 23:25:12 2017
@@ -800,8 +800,12 @@ def fencoding_EQ : Joined<["-"], "fencod
 def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, 
Flags<[CoreOption]>;
 def fexceptions : Flag<["-"], "fexceptions">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable support for exception handling">;
+def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
 def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group,
   Flags<[CC1Option]>, HelpText<"Use SjLj 

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

I also want to note a small addition armb and thumbeb for NetBSD.
They were missed in the previous version.
Just waiting for tests to pass now.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

@mstorsjo yup, I added a comment on the commit about the failure here 
https://reviews.llvm.org/rL319294
I have already fixed both issues.
Will re apply shortly.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp:368
+getTriple().getArch() == llvm::Triple::thumb)
+  return llvm::ExceptionHandling::SjLj;
   case llvm::Triple::GNUEABIHF:

This seems to need a fallthrough comment to build without warnings in some 
configurations, see e.g. 
http://lab.llvm.org:8011/builders/ubuntu-gcc7.1-werror/builds/3329/steps/build-unified-tree/logs/stdio


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChain.cpp:457
+  if (Triple.isOSWindows())
+return llvm::ExceptionHandling::WinEH;
+  return llvm::ExceptionHandling::None;

It looks like this broke some buildbot after all - see e.g. 
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/20371.

Perhaps it's best to just have the default here be None even for Windows, and 
just let it be overridden for MinGW? I don't think we did define `__SEH__` at 
all when targeting MSVC before either, so it should be fine to just leave it 
unset and not override anything from the backend defaults here.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

r319295 - Revert "Toolchain: Normalize dwarf, sjlj and seh eh"

[Martell Malone via cfe-commits]

Author: martell
Date: Tue Nov 28 22:51:27 2017
New Revision: 319295

URL: http://llvm.org/viewvc/llvm-project?rev=319295=rev
Log:
Revert "Toolchain: Normalize dwarf, sjlj and seh eh"

This reverts rL319294.
The windows sanitizer does not like seh on x86.
Will re apply with None type for x86

Removed:
cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.h
cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.h
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
cfe/trunk/test/Preprocessor/arm-target-features.c
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=319295=319294=319295=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 22:51:27 2017
@@ -1706,18 +1706,10 @@ Which overload candidates to show when o
 
 Enable C++14 sized global deallocation functions
 
-.. option:: -fdwarf-exceptions
-
-Use DWARF style exceptions
-
 .. option:: -fsjlj-exceptions
 
 Use SjLj style exceptions
 
-.. option:: -fseh-exceptions
-
-Use SEH style exceptions
-
 .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
 
 Enable the superword-level parallelism vectorization passes

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=319295=319294=319295=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Tue Nov 28 22:51:27 2017
@@ -124,9 +124,7 @@ LANGOPT(ZVector   , 1, 0, "Syste
 LANGOPT(Exceptions, 1, 0, "exception handling")
 LANGOPT(ObjCExceptions, 1, 0, "Objective-C exceptions")
 LANGOPT(CXXExceptions , 1, 0, "C++ exceptions")
-LANGOPT(DWARFExceptions   , 1, 0, "dwarf exception handling")
 LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling")
-LANGOPT(SEHExceptions , 1, 0, "SEH .xdata exception handling")
 LANGOPT(ExternCNoUnwind   , 1, 0, "Assume extern C functions don't unwind")
 LANGOPT(TraditionalCPP, 1, 0, "traditional CPP emulation")
 LANGOPT(RTTI  , 1, 1, "run-time type information")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=319295=319294=319295=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Nov 28 22:51:27 2017
@@ -800,12 +800,8 @@ def fencoding_EQ : Joined<["-"], "fencod
 def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, 
Flags<[CoreOption]>;
 def fexceptions : Flag<["-"], "fexceptions">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable support for exception handling">;
-def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group,
-  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
 def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group,
   Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">;
-def fseh_exceptions : Flag<["-"], "fseh-exceptions">, Group,
-  Flags<[CC1Option]>, HelpText<"Use SEH style exceptions">;
 def fexcess_precision_EQ : Joined<["-"], "fexcess-precision=">,
 Group;
 def : Flag<["-"], "fexpensive-optimizations">, 
Group;

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=319295=319294=319295=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Tue Nov 28 22:51:27 2017
@@ -397,9 +397,10 @@ public:
 return llvm::DebuggerKind::GDB;
   }
 
-  /// 

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319294: Toolchain: Normalize dwarf, sjlj and seh eh 
(authored by martell).

Changed prior to commit:
  https://reviews.llvm.org/D39673?vs=124695=124696#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39673

Files:
  cfe/trunk/docs/ClangCommandLineReference.rst
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/ToolChain.h
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/lib/Basic/Targets/OSTargets.cpp
  cfe/trunk/lib/Basic/Targets/OSTargets.h
  cfe/trunk/lib/Basic/Targets/X86.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CGException.cpp
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/lib/Driver/ToolChains/Darwin.h
  cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
  cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
  cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
  cfe/trunk/lib/Driver/ToolChains/MinGW.h
  cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
  cfe/trunk/lib/Driver/ToolChains/NetBSD.h
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
  cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
  cfe/trunk/test/Preprocessor/arm-target-features.c
  cfe/trunk/test/Preprocessor/init.c

Index: cfe/trunk/include/clang/Driver/ToolChain.h
===
--- cfe/trunk/include/clang/Driver/ToolChain.h
+++ cfe/trunk/include/clang/Driver/ToolChain.h
@@ -397,10 +397,9 @@
 return llvm::DebuggerKind::GDB;
   }
 
-  /// UseSjLjExceptions - Does this tool chain use SjLj exceptions.
-  virtual bool UseSjLjExceptions(const llvm::opt::ArgList ) const {
-return false;
-  }
+  /// GetExceptionModel - Return the tool chain exception model.
+  virtual llvm::ExceptionHandling
+  GetExceptionModel(const llvm::opt::ArgList ) const;
 
   /// SupportsEmbeddedBitcode - Does this tool chain support embedded bitcode.
   virtual bool SupportsEmbeddedBitcode() const {
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -800,8 +800,12 @@
 def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, Flags<[CoreOption]>;
 def fexceptions : Flag<["-"], "fexceptions">, Group, Flags<[CC1Option]>,
   HelpText<"Enable support for exception handling">;
+def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
 def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group,
   Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">;
+def fseh_exceptions : Flag<["-"], "fseh-exceptions">, Group,
+  Flags<[CC1Option]>, HelpText<"Use SEH style exceptions">;
 def fexcess_precision_EQ : Joined<["-"], "fexcess-precision=">,
 Group;
 def : Flag<["-"], "fexpensive-optimizations">, Group;
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -124,7 +124,9 @@
 LANGOPT(Exceptions, 1, 0, "exception handling")
 LANGOPT(ObjCExceptions, 1, 0, "Objective-C exceptions")
 LANGOPT(CXXExceptions , 1, 0, "C++ exceptions")
+LANGOPT(DWARFExceptions   , 1, 0, "dwarf exception handling")
 LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling")
+LANGOPT(SEHExceptions , 1, 0, "SEH .xdata exception handling")
 LANGOPT(ExternCNoUnwind   , 1, 0, "Assume extern C functions don't unwind")
 LANGOPT(TraditionalCPP, 1, 0, "traditional CPP emulation")
 LANGOPT(RTTI  , 1, 1, "run-time type information")
Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -1442,6 +1442,7 @@
 //
 // ARM-MSVC: #define _M_ARM_NT 1
 // ARM-MSVC: #define _WIN32 1
+// ARM-MSVC-NOT:#define __ARM_DWARF_EH__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=aarch64-windows-msvc < /dev/null | FileCheck -match-full-lines -check-prefix AARCH64-MSVC %s
 //
Index: cfe/trunk/test/Preprocessor/arm-target-features.c
===
--- cfe/trunk/test/Preprocessor/arm-target-features.c
+++ cfe/trunk/test/Preprocessor/arm-target-features.c
@@ -214,6 +214,7 @@
 // A5:#define __ARM_ARCH_7A__ 1
 // A5-NOT:#define __ARM_ARCH_EXT_IDIV__
 // A5:#define __ARM_ARCH_PROFILE 'A'
+// A5-NOT:#define __ARM_DWARF_EH__ 1
 // A5-NOT: #define __ARM_FEATURE_DIRECTED_ROUNDING
 // A5:#define __ARM_FEATURE_DSP 1
 // A5-NOT: #define __ARM_FEATURE_NUMERIC_MAXMIN
@@ -225,6 +226,7 @@
 // 

r319294 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via cfe-commits]

Author: martell
Date: Tue Nov 28 22:25:13 2017
New Revision: 319294

URL: http://llvm.org/viewvc/llvm-project?rev=319294=rev
Log:
Toolchain: Normalize dwarf, sjlj and seh eh

adds -fseh-exceptions and -fdwarf-exceptions flags

clang will check if the user has specified an exception model flag,
in the absense of specifying the exception model clang will then check
the driver default and append the model flag for that target to cc1

clang cc1 assumes dwarf is the default if none is passed
and -fno-exceptions has a higher priority then specifying the model

move __SEH__ macro definitions out of Targets into InitPreprocessor
behind the -fseh-exceptions flag

move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check

remove unused USESEHExceptions from the MinGW Driver

fold USESjLjExceptions into a new GetExceptionModel function that
gives the toolchain classes more flexibility with eh models

Reviewers: rnk, mstorsjo

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

Added:
cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.h
cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.h
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
cfe/trunk/test/Preprocessor/arm-target-features.c
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=319294=319293=319294=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 22:25:13 2017
@@ -1706,10 +1706,18 @@ Which overload candidates to show when o
 
 Enable C++14 sized global deallocation functions
 
+.. option:: -fdwarf-exceptions
+
+Use DWARF style exceptions
+
 .. option:: -fsjlj-exceptions
 
 Use SjLj style exceptions
 
+.. option:: -fseh-exceptions
+
+Use SEH style exceptions
+
 .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
 
 Enable the superword-level parallelism vectorization passes

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=319294=319293=319294=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Tue Nov 28 22:25:13 2017
@@ -124,7 +124,9 @@ LANGOPT(ZVector   , 1, 0, "Syste
 LANGOPT(Exceptions, 1, 0, "exception handling")
 LANGOPT(ObjCExceptions, 1, 0, "Objective-C exceptions")
 LANGOPT(CXXExceptions , 1, 0, "C++ exceptions")
+LANGOPT(DWARFExceptions   , 1, 0, "dwarf exception handling")
 LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling")
+LANGOPT(SEHExceptions , 1, 0, "SEH .xdata exception handling")
 LANGOPT(ExternCNoUnwind   , 1, 0, "Assume extern C functions don't unwind")
 LANGOPT(TraditionalCPP, 1, 0, "traditional CPP emulation")
 LANGOPT(RTTI  , 1, 1, "run-time type information")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=319294=319293=319294=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Nov 28 22:25:13 2017
@@ -800,8 +800,12 @@ def fencoding_EQ : Joined<["-"], "fencod
 def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, 
Flags<[CoreOption]>;
 def fexceptions : Flag<["-"], "fexceptions">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable support for exception handling">;
+def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
 def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group,
   

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

landing this as it was previous approved by reid in this state and again re 
checked by martin.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell updated this revision to Diff 124695.
martell added a comment.

- disregard my last comment, lets go with seh on all windows targets.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Basic/Targets/ARM.cpp
  lib/Basic/Targets/OSTargets.cpp
  lib/Basic/Targets/OSTargets.h
  lib/Basic/Targets/X86.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGException.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/FreeBSD.h
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/NetBSD.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/CodeGenCXX/mingw-w64-exceptions.c
  test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
  test/Preprocessor/arm-target-features.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -1442,6 +1442,7 @@
 //
 // ARM-MSVC: #define _M_ARM_NT 1
 // ARM-MSVC: #define _WIN32 1
+// ARM-MSVC-NOT:#define __ARM_DWARF_EH__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=aarch64-windows-msvc < /dev/null | FileCheck -match-full-lines -check-prefix AARCH64-MSVC %s
 //
Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -214,6 +214,7 @@
 // A5:#define __ARM_ARCH_7A__ 1
 // A5-NOT:#define __ARM_ARCH_EXT_IDIV__
 // A5:#define __ARM_ARCH_PROFILE 'A'
+// A5-NOT:#define __ARM_DWARF_EH__ 1
 // A5-NOT: #define __ARM_FEATURE_DIRECTED_ROUNDING
 // A5:#define __ARM_FEATURE_DSP 1
 // A5-NOT: #define __ARM_FEATURE_NUMERIC_MAXMIN
@@ -225,6 +226,7 @@
 // A7:#define __ARM_ARCH 7
 // A7:#define __ARM_ARCH_EXT_IDIV__ 1
 // A7:#define __ARM_ARCH_PROFILE 'A'
+// A7-NOT:#define __ARM_DWARF_EH__ 1
 // A7:#define __ARM_FEATURE_DSP 1
 // A7:#define __ARM_FP 0xE
 
Index: test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
===
--- test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
+++ test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 %s -fexceptions -fseh-exceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 %s -fexceptions -fdwarf-exceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
 
 extern "C" void foo();
Index: test/CodeGenCXX/mingw-w64-exceptions.c
===
--- /dev/null
+++ test/CodeGenCXX/mingw-w64-exceptions.c
@@ -0,0 +1,22 @@
+// RUN: %clang -target x86_64-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SEH
+// RUN: %clang -target i686-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -fseh-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SEH
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// CHECK-SEH: "-fseh-exceptions"
+// CHECK-SJLJ: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fseh-exceptions"
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -677,8 +677,14 @@
 Builder.defineMacro("__EXCEPTIONS");
   if (!LangOpts.MSVCCompat && LangOpts.RTTI)
 Builder.defineMacro("__GXX_RTTI");
+
   if (LangOpts.SjLjExceptions)
 Builder.defineMacro("__USING_SJLJ_EXCEPTIONS__");
+  else if (LangOpts.SEHExceptions)
+Builder.defineMacro("__SEH__");
+  else if (LangOpts.DWARFExceptions &&
+  (TI.getTriple().isThumb() || TI.getTriple().isARM()))
+Builder.defineMacro("__ARM_DWARF_EH__");
 
   if (LangOpts.Deprecated)
 

[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D40508#938675, @sepavloff wrote:

> In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:
>
> > My skepticism about the raw_ostream is not about the design of having a 
> > custom raw_ostream subclass, it's that that subclass could conceivably be 
> > re-used by some other client.  It feels like it belongs as an internal hack 
> > in Clang absent some real evidence that someone else would use it.
>
>
> This class can be helpful in various cases where string identifier must 
> persistently identify an object and memory consumption must be low. It may be:
>
> - Name mangling,
> - Symbol obfuscation,
> - More convenient replacement for `raw_sha1_ostream` (the latter produces 
> binary result, while `raw_abbrev_ostream` produces text),


There's no requirement to persistently identify an object here.

> - If we introduce an option that allows a user to specify how many symbols of 
> full type name are kept in abbreviated name, then `llvm-link` may see types 
> named as `foo` and `2544896211ff669ed44dccd265f4c9163b340190`, if they 
> come from modules compiled with different options. To find out that these are 
> the same type, it must have access to the same machinery.

The LLVM linking model does not actually depend on struct type names matching.  
My understanding is that, at best, it uses that as a heuristic for deciding 
whether to make an effort to unify two types, but it's not something that's 
ultimately supposed to impact IR semantics.

If we needed IR type names to match reliably, we would use a mangled name, not 
a pretty-print.


https://reviews.llvm.org/D40508



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell added inline comments.



Comment at: lib/Driver/ToolChain.cpp:458
+if (Triple.getArch() == llvm::Triple::x86)
+  return llvm::ExceptionHandling::DwarfCFI;
+else

mstorsjo wrote:
> I'd suggest braces around the outer if statement.
> 
> But is there any point to this default fallback here at all? Dwarf isn't 
> right for x86 for MSVC, and we set the right defaults for all the MinGW cases 
> in that driver in any case. So isn't it enough to just return None always 
> here, or possibly WinEH for all windows cases?
Won't need braces with the change I am about to make.

The previous check was `getArch() == llvm::Triple::x86_64` then use seh.
This didn't take into account arm or aarch64 so I flipped this.

I read the wrong llvm class for  `llvm::Triple::x86` `X86MCAsmInfoGNUCOFF`
It should have been `X86MCAsmInfoMicrosoft` I looked at.
```
  if (Triple.getArch() == Triple::x86_64) {
PrivateGlobalPrefix = ".L";
PrivateLabelPrefix = ".L";
CodePointerSize = 8;
WinEHEncodingType = WinEH::EncodingType::Itanium;
  } else {
// 32-bit X86 doesn't use CFI, so this isn't a real encoding type. It's just
// a place holder that the Windows EHStreamer looks for to suppress CFI
// output. In particular, usesWindowsCFI() returns false.
WinEHEncodingType = WinEH::EncodingType::X86;
  }

  ExceptionsType = ExceptionHandling::WinEH;
```
I think None in the case of x86 is the behavior that was present previously so 
I am going to revert from dwarf to none.

https://www.google.com/patents/US5628016
That borland patent has long since expired now and seh is the default in llvm 
for x86 so we can do seh on x86 for clang but I will leave that to a different 
patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39455: [CodeGen] Add initial support for union members in TBAA

[Hal Finkel via Phabricator via cfe-commits]

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

I think this looks fine (and I agree that the underlying premise makes sense). 
Before you commit, could you also please add some tests with nested union 
members?


Repository:
  rL LLVM

https://reviews.llvm.org/D39455



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

[PATCH] D40508: Replace long type names in IR with hashes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:

> My skepticism about the raw_ostream is not about the design of having a 
> custom raw_ostream subclass, it's that that subclass could conceivably be 
> re-used by some other client.  It feels like it belongs as an internal hack 
> in Clang absent some real evidence that someone else would use it.


This class can be helpful in various cases where string identifier must 
persistently identify an object and memory consumption must be low. It may be:

- Name mangling,
- Symbol obfuscation,
- More convenient replacement for `raw_sha1_ostream` (the latter produces 
binary result, while `raw_abbrev_ostream` produces text),
- If we introduce an option that allows a user to specify how many symbols of 
full type name are kept in abbreviated name, then `llvm-link` may see types 
named as `foo` and `2544896211ff669ed44dccd265f4c9163b340190`, if they 
come from modules compiled with different options. To find out that these are 
the same type, it must have access to the same machinery.




Comment at: lib/AST/TypePrinter.cpp:1532-1534
+namespace {
+template
+void printTo(raw_ostream , ArrayRef Args, const PrintingPolicy ,

rjmccall wrote:
> sepavloff wrote:
> > rnk wrote:
> > > `static` is nicer than a short anonymous namespace.
> > Yes, but this is function template. It won't create symbol in object file. 
> > Actually anonymous namespace has no effect here, it is only a documentation 
> > hint.
> Nonetheless, we generally prefer to use 'static' on internal functions, even 
> function templates, instead of putting them in anonymous namespaces.
OK, fixed in rL319290.


https://reviews.llvm.org/D40508



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: lib/Driver/ToolChain.cpp:458
+if (Triple.getArch() == llvm::Triple::x86)
+  return llvm::ExceptionHandling::DwarfCFI;
+else

I'd suggest braces around the outer if statement.

But is there any point to this default fallback here at all? Dwarf isn't right 
for x86 for MSVC, and we set the right defaults for all the MinGW cases in that 
driver in any case. So isn't it enough to just return None always here, or 
possibly WinEH for all windows cases?


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via Phabricator via cfe-commits]

martell updated this revision to Diff 124692.
martell added a comment.

address reid's comment
update to HEAD


Repository:
  rL LLVM

https://reviews.llvm.org/D39673

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Basic/Targets/ARM.cpp
  lib/Basic/Targets/OSTargets.cpp
  lib/Basic/Targets/OSTargets.h
  lib/Basic/Targets/X86.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGException.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/FreeBSD.h
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/NetBSD.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/CodeGenCXX/mingw-w64-exceptions.c
  test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
  test/Preprocessor/arm-target-features.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -1442,6 +1442,7 @@
 //
 // ARM-MSVC: #define _M_ARM_NT 1
 // ARM-MSVC: #define _WIN32 1
+// ARM-MSVC-NOT:#define __ARM_DWARF_EH__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=aarch64-windows-msvc < /dev/null | FileCheck -match-full-lines -check-prefix AARCH64-MSVC %s
 //
Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -214,6 +214,7 @@
 // A5:#define __ARM_ARCH_7A__ 1
 // A5-NOT:#define __ARM_ARCH_EXT_IDIV__
 // A5:#define __ARM_ARCH_PROFILE 'A'
+// A5-NOT:#define __ARM_DWARF_EH__ 1
 // A5-NOT: #define __ARM_FEATURE_DIRECTED_ROUNDING
 // A5:#define __ARM_FEATURE_DSP 1
 // A5-NOT: #define __ARM_FEATURE_NUMERIC_MAXMIN
@@ -225,6 +226,7 @@
 // A7:#define __ARM_ARCH 7
 // A7:#define __ARM_ARCH_EXT_IDIV__ 1
 // A7:#define __ARM_ARCH_PROFILE 'A'
+// A7-NOT:#define __ARM_DWARF_EH__ 1
 // A7:#define __ARM_FEATURE_DSP 1
 // A7:#define __ARM_FP 0xE
 
Index: test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
===
--- test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
+++ test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 %s -fexceptions -fseh-exceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 %s -fexceptions -fdwarf-exceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
 
 extern "C" void foo();
Index: test/CodeGenCXX/mingw-w64-exceptions.c
===
--- /dev/null
+++ test/CodeGenCXX/mingw-w64-exceptions.c
@@ -0,0 +1,22 @@
+// RUN: %clang -target x86_64-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SEH
+// RUN: %clang -target i686-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -fseh-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SEH
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// CHECK-SEH: "-fseh-exceptions"
+// CHECK-SJLJ: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fseh-exceptions"
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -677,8 +677,14 @@
 Builder.defineMacro("__EXCEPTIONS");
   if (!LangOpts.MSVCCompat && LangOpts.RTTI)
 Builder.defineMacro("__GXX_RTTI");
+
   if (LangOpts.SjLjExceptions)
 Builder.defineMacro("__USING_SJLJ_EXCEPTIONS__");
+  else if (LangOpts.SEHExceptions)
+Builder.defineMacro("__SEH__");
+  else if (LangOpts.DWARFExceptions &&
+  (TI.getTriple().isThumb() || TI.getTriple().isARM()))
+Builder.defineMacro("__ARM_DWARF_EH__");
 
   if (LangOpts.Deprecated)
 Builder.defineMacro("__DEPRECATED");
Index: 

r319290 - Use static function instead of anonymous namespace

[Serge Pavlov via cfe-commits]

Author: sepavloff
Date: Tue Nov 28 21:10:11 2017
New Revision: 319290

URL: http://llvm.org/viewvc/llvm-project?rev=319290=rev
Log:
Use static function instead of anonymous namespace

Modified:
cfe/trunk/lib/AST/TypePrinter.cpp

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=319290=319289=319290=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Tue Nov 28 21:10:11 2017
@@ -1529,10 +1529,9 @@ static const TemplateArgument 
   return A.getArgument();
 }
 
-namespace {
 template
-void printTo(raw_ostream , ArrayRef Args, const PrintingPolicy ,
- bool SkipBrackets) {
+static void printTo(raw_ostream , ArrayRef Args,
+const PrintingPolicy , bool SkipBrackets) {
   const char *Comma = Policy.MSVCFormatting ? "," : ", ";
   if (!SkipBrackets)
 OS << '<';
@@ -1576,7 +1575,6 @@ void printTo(raw_ostream , ArrayRef';
 }
-}
 
 void clang::printTemplateArgumentList(raw_ostream ,
   const TemplateArgumentListInfo ,


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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1173
+H.range = L.range;
+Ref = getDataBufferFromSourceRange(AST, SR, L);
+  }

malaperle wrote:
> I get the same crash as I mentioned before if I hover on the class in 
> "isa(D)", from Clangdunit.cpp (disclaimer: I'm testing with 
> older source so it might not be there anymore.)
> 
> I have still yet to investigate this.
I think it has to do with macro expansions, the plot thickens..


https://reviews.llvm.org/D35894



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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.



Comment at: clangd/ClangdLSPServer.cpp:245
+
+  C.reply(Hover::unparse(H->Value));
+}

we need to check the "Expected" here, so

  if (!H) {
C.replyError(ErrorCode::InternalError,
 llvm::toString(H.takeError()));
return;
  }

Unless you can think of other error situations?



Comment at: clangd/ClangdServer.cpp:448
   });
+
   return make_tagged(std::move(Result), TaggedFS.Tag);

revert



Comment at: clangd/ClangdServer.cpp:450
 }
-
 llvm::Optional ClangdServer::switchSourceHeader(PathRef Path) {

revert



Comment at: clangd/ClangdUnit.cpp:943
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationLocationsFinder : public index::IndexDataConsumer {
-  std::vector DeclarationLocations;
+/// Finds declarations  and macros that a given source location refers to.
+class DeclarationAndMacrosFinder : public index::IndexDataConsumer {

extra space before the "and"



Comment at: clangd/ClangdUnit.cpp:1092
+  // Default Hover values
+  std::vector EmptyVector;
+  Hover H;

not used, remove



Comment at: clangd/ClangdUnit.cpp:1094
+  Hover H;
+  StringRef Ref;
+

Can this be named something more descriptive? HoverText?



Comment at: clangd/ClangdUnit.cpp:1117
+
+  if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];

I think we need to simplify this part a bit. I'll try to find a way. Feel free 
to wait until more comments before updating.



Comment at: clangd/ClangdUnit.cpp:1173
+H.range = L.range;
+Ref = getDataBufferFromSourceRange(AST, SR, L);
+  }

I get the same crash as I mentioned before if I hover on the class in 
"isa(D)", from Clangdunit.cpp (disclaimer: I'm testing with 
older source so it might not be there anymore.)

I have still yet to investigate this.



Comment at: clangd/ClangdUnit.cpp:1238
+/// Returns the file buffer data that the given SourceRange points to.
+StringRef clangd::getDataBufferFromSourceRange(ParsedAST , SourceRange SR,
+   Location L) {

getBufferDataFromSourceRange, to be consistent with 
getSourceManager().getBufferData



Comment at: clangd/ClangdUnit.cpp:1239
+StringRef clangd::getDataBufferFromSourceRange(ParsedAST , SourceRange SR,
+   Location L) {
+  StringRef Ref;

I think the SourceRange SR should be used here, not Location.



Comment at: clangd/ClangdUnit.h:318
+
+std::string getScope(const NamedDecl *ND, const LangOptions );
+

I think this can be only in the source file, in a an anonymous namespace.



Comment at: clangd/Protocol.cpp:1029
+{"contents", json::ary(H.contents)},
+{"range", DefaultRange},
+};

I don't think "range" should be outputted here



Comment at: test/clangd/hover.test:10
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define
 MACRO 1\nnamespace ns1 {\nint test = 5;\nstruct MyClass {\nint xasd;\nvoid 
anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};}\nint 
main() {\nint a;\na;\nint b = ns1::test;\nns1::MyClass* 
Params;\nParams->anotherOperation();\nMACRO;}\n"}}}
+

probably the test should include templates and comments?



Comment at: test/clangd/hover.test:15
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":0,"character":12}}}
+# Go to local variable
+# CHECK: 
{"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"#define
 MACRO 
1"}],"range":{"end":{"character":15,"line":0},"start":{"character":8,"line":0

copy/pasted comment?



Comment at: test/clangd/hover.test:21
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":14,"character":1}}}
+# Go to local variable
+# CHECK: 
{"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"int 
a"}],"range":{"end":{"character":5,"line":13},"start":{"character":0,"line":13

copy/pasted comment?



Comment at: test/clangd/hover.test:27
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":15,"character":15}}}
+# Go to local variable
+# CHECK: 

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc updated this revision to Diff 124691.
kcc added a comment.

mention alternatives for memory access instrumentation


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst

Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,125 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** or TASAN
+(alternative: **HardwareAssistedAddressSanitizer** or HWASAN)
+and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function.
+The function encodes the type and the size of access in its name;
+it receives the address as a parameter, e.g. `__hwams_load4(void *ptr)`;
+it loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` (or calls `__hwams_error_load4(void *ptr)`) on mismatch.
+
+It's possible to inline this callback too.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * May require changes in the OS kernels (e.g. Linux seems to dislike
+tagged pointers passed from address space).
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

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

rsmith added inline comments.



Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

erichkeane wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > This isn't quite right; you want min(bitfield length, bit size of type) 
> > > here. Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.
> > I guess I'm missing something with this comment... why would a bitfield 
> > with bool b: 8 have padding?
> > 
> > Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an 
> > int)?
> > Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 9, 
> > but Clang seems to allow 'bool' to have ANY size.  Is that an error itself?
> > 
> > Finally: Should we consider ANY bool to be not a unique object 
> > representation?  It has 1 bit of data, but 8 bits of storage.  GCC's 
> > implementation of this trait accepts them, but I'm second guessing that at 
> > the moment...
> I did a bit more looking into this... In the 'TYPE b: X' case, 'X' is always 
> the size of the field, so :
> 
> struct Bitfield {
>   bool b: 16;
> };
> static_assert(sizeof(Bitfield) == 2);
> 
> The rest of my padding detection works correctly.
Sounds like you've found more GCC bugs. In C++ (unlike in C), there is no 
restriction on the maximum length of a bit-field. Specifically, [class.bit]p1 
says: "The value of the integral constant expression may be larger than the 
number of bits in the object
representation (6.7) of the bit-field’s type; in such cases the extra bits are 
padding bits (6.7)."

For non-`bool` bit-fields, or `bool` bit-fields with more than 8 bits, the 
extra bits are padding, and the type does not have unique object 
representations. You'll need to check for this.

For `bool` bit-fields of length 1, we obviously have unique object 
representations for that one bit.

The interesting case is `bool` bit-fields with length between 2 and 8 
inclusive. It would seem reasonable to assume they have unique representations, 
as we do for plain `bool` values, but this is really an ABI question (as, 
actually, is the plain `bool` case). The x86-64 psABI is not very clear on the 
bit-field question, but it does say "Booleans, when stored in a memory object, 
are stored as single byte objects the value of which is always 0 (false) or 1 
(true).", so for at least x86-64, `bool`s of up to 8 bits should be considered 
to have unique object representations. The PPC64 psABI appears to say the same 
thing, but is also somewhat unclear about the bit-field case.



Comment at: lib/AST/ASTContext.cpp:2277
+
+  return false;
+}

More cases to handle here:

 * vectors (careful about, eg, vector of 3 foo)
 * `_Complex int` and friends
 * `_Atomic T`
 * Obj-C block pointers
 * Obj-C object pointers
 * and perhaps OpenCL's various builtin types (pipe, sampler_t, event_t, 
clk_event_t, queue_t, reserve_id_t)

It's fine to leave these for another change, but we shouldn't forget about 
them. (There're also Obj-C class types and the Obj-C selector type, but I think 
it makes sense for those to return `false` here.)


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Did ore looking into the bitfield shennangins.  Will update this patch 
tomorrow.  @rsmith if you get a chance, do you perhaps have an opinion on 
'bool' fields?  It seems to me that a bool could either be considered an 8 bit 
value, or a 1 bit value with 7 bits of padding.  So:

  bool b;
  struct S { bool b; };
   // Patch currently lists these as unique.  Should they be: 
  static_assert(!__has_unique_object_representations(b));
  static_assert(!__has_unique_object_representations(S));




Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

erichkeane wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > This isn't quite right; you want min(bitfield length, bit size of type) 
> > > here. Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.
> > I guess I'm missing something with this comment... why would a bitfield 
> > with bool b: 8 have padding?
> > 
> > Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an 
> > int)?
> > Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 9, 
> > but Clang seems to allow 'bool' to have ANY size.  Is that an error itself?
> > 
> > Finally: Should we consider ANY bool to be not a unique object 
> > representation?  It has 1 bit of data, but 8 bits of storage.  GCC's 
> > implementation of this trait accepts them, but I'm second guessing that at 
> > the moment...
> I did a bit more looking into this... In the 'TYPE b: X' case, 'X' is always 
> the size of the field, so :
> 
> struct Bitfield {
>   bool b: 16;
> };
> static_assert(sizeof(Bitfield) == 2);
> 
> The rest of my padding detection works correctly.
Umm... so holy crap.  We actually reserve the object spece for the whole thing, 
but ONLY the first sizeof(field) fields are actually stored to!  Everything 
else is truncated!

I think the correct solution is to do:
   // too large of a bitfield size causes truncation, and thus 'padding' bits.
   if (FieldSizeInBits > Context.getTypeSizeInBits(Field->getType()) return 
false;


https://reviews.llvm.org/D39347



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

[libunwind] r319287 - Creating release candidate rc2 from release_501 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Tue Nov 28 18:49:31 2017
New Revision: 319287

URL: http://llvm.org/viewvc/llvm-project?rev=319287=rev
Log:
Creating release candidate rc2 from release_501 branch

Added:
libunwind/tags/RELEASE_501/rc2/   (props changed)
  - copied from r319286, libunwind/branches/release_50/

Propchange: libunwind/tags/RELEASE_501/rc2/
--
svn:mergeinfo = /libunwind/trunk:308871,309147


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

[libcxxabi] r319281 - Creating release candidate rc2 from release_501 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Tue Nov 28 18:49:04 2017
New Revision: 319281

URL: http://llvm.org/viewvc/llvm-project?rev=319281=rev
Log:
Creating release candidate rc2 from release_501 branch

Added:
libcxxabi/tags/RELEASE_501/rc2/
  - copied from r319280, libcxxabi/branches/release_50/

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

[libcxx] r319280 - Creating release candidate rc2 from release_501 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Tue Nov 28 18:49:00 2017
New Revision: 319280

URL: http://llvm.org/viewvc/llvm-project?rev=319280=rev
Log:
Creating release candidate rc2 from release_501 branch

Added:
libcxx/tags/RELEASE_501/rc2/   (props changed)
  - copied from r319279, libcxx/branches/release_50/

Propchange: libcxx/tags/RELEASE_501/rc2/
--
--- svn:mergeinfo (added)
+++ svn:mergeinfo Tue Nov 28 18:49:00 2017
@@ -0,0 +1,2 @@
+/libcxx/branches/apple:136569-137939
+/libcxx/trunk:309296,309307,309474,309838,309851,309917,309920


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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/string_view:577
+   bool starts_with(basic_string_view __s) const _NOEXCEPT
+   { return size() >= __s.size() && compare(0, __s.size(), __s) == 0; }
+

rsmith wrote:
> mclow.lists wrote:
> > rsmith wrote:
> > > Is this a conforming implementation? The `size()` check isn't part of the 
> > > specification, and `compare` could run arbitrary user-supplied code, so 
> > > the absence of a call to it seems observable.
> > > 
> > > Don't get me wrong: I think this check is the right thing to do, and it 
> > > only makes a difference if `traits::compare` has observable side-effects. 
> > > But we should file a bug against the standard to get this check added 
> > > there.
> > The code as specified in the paper has a bug in it ;-)
> > I'll be filing an LWG issue once we have a draft standard with these bits 
> > in it.
> You can get a copy of the new draft a few days early from here: 
> https://github.com/cplusplus/draft/blob/master/papers/n4713.pdf
Thanks. I can also build it myself; but I'd like to refer to it in the issue, 
so I can wait a day or two.


https://reviews.llvm.org/D40586



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124679.
erichkeane added a comment.

thanks for the comments rsmith, think this is all caught up to your comments.  
Also added the test for member ptr differences.


https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/has_unique_object_reps_member_ptr.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext ) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

erichkeane wrote:
> rsmith wrote:
> > This isn't quite right; you want min(bitfield length, bit size of type) 
> > here. Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.
> I guess I'm missing something with this comment... why would a bitfield with 
> bool b: 8 have padding?
> 
> Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an int)?
> Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 9, 
> but Clang seems to allow 'bool' to have ANY size.  Is that an error itself?
> 
> Finally: Should we consider ANY bool to be not a unique object 
> representation?  It has 1 bit of data, but 8 bits of storage.  GCC's 
> implementation of this trait accepts them, but I'm second guessing that at 
> the moment...
I did a bit more looking into this... In the 'TYPE b: X' case, 'X' is always 
the size of the field, so :

struct Bitfield {
  bool b: 16;
};
static_assert(sizeof(Bitfield) == 2);

The rest of my padding detection works correctly.


https://reviews.llvm.org/D39347



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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

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

rsmith added inline comments.



Comment at: include/string_view:577
+   bool starts_with(basic_string_view __s) const _NOEXCEPT
+   { return size() >= __s.size() && compare(0, __s.size(), __s) == 0; }
+

mclow.lists wrote:
> rsmith wrote:
> > Is this a conforming implementation? The `size()` check isn't part of the 
> > specification, and `compare` could run arbitrary user-supplied code, so the 
> > absence of a call to it seems observable.
> > 
> > Don't get me wrong: I think this check is the right thing to do, and it 
> > only makes a difference if `traits::compare` has observable side-effects. 
> > But we should file a bug against the standard to get this check added there.
> The code as specified in the paper has a bug in it ;-)
> I'll be filing an LWG issue once we have a draft standard with these bits in 
> it.
You can get a copy of the new draft a few days early from here: 
https://github.com/cplusplus/draft/blob/master/papers/n4713.pdf


https://reviews.llvm.org/D40586



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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists updated this revision to Diff 124677.
mclow.lists added a comment.

Wrapped the `string_view` bits in #ifdef for C++2a.
De-tabbed.


https://reviews.llvm.org/D40586

Files:
  include/string
  include/string_view
  test/std/strings/basic.string/string.ends_with/ends_with.char.pass.cpp
  test/std/strings/basic.string/string.ends_with/ends_with.ptr.pass.cpp
  test/std/strings/basic.string/string.ends_with/ends_with.string_view.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
  test/std/strings/basic.string/string.starts_with/starts_with.char.pass.cpp
  test/std/strings/basic.string/string.starts_with/starts_with.ptr.pass.cpp
  
test/std/strings/basic.string/string.starts_with/starts_with.string_view.pass.cpp
  test/std/strings/string.view/string.view.template/ends_with.char.pass.cpp
  test/std/strings/string.view/string.view.template/ends_with.ptr.pass.cpp
  
test/std/strings/string.view/string.view.template/ends_with.string_view.pass.cpp
  test/std/strings/string.view/string.view.template/starts_with.char.pass.cpp
  test/std/strings/string.view/string.view.template/starts_with.ptr.pass.cpp
  
test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp

Index: test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
===
--- test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
+++ test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
@@ -0,0 +1,104 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// 
+
+//   constexpr bool starts_with(string_view x) const noexcept;
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "constexpr_char_traits.hpp"
+
+int main()
+{
+{
+typedef std::string_view SV;
+const char *s = "abcde";
+SV  sv0 {};
+SV  sv1 { s, 1 };
+SV  sv2 { s, 2 };
+SV  sv3 { s, 3 };
+SV  sv4 { s, 4 };
+SV  sv5 { s, 5 };
+SV  svNot {"def", 3 };
+
+ASSERT_NOEXCEPT(sv0.starts_with(sv0));
+
+assert ( sv0.starts_with(sv0));
+assert (!sv0.starts_with(sv1));
+
+assert ( sv1.starts_with(sv0));
+assert ( sv1.starts_with(sv1));
+assert (!sv1.starts_with(sv2));
+assert (!sv1.starts_with(sv3));
+assert (!sv1.starts_with(sv4));
+assert (!sv1.starts_with(sv5));
+assert (!sv1.starts_with(svNot));
+
+assert ( sv2.starts_with(sv0));
+assert ( sv2.starts_with(sv1));
+assert ( sv2.starts_with(sv2));
+assert (!sv2.starts_with(sv3));
+assert (!sv2.starts_with(sv4));
+assert (!sv2.starts_with(sv5));
+assert (!sv2.starts_with(svNot));
+
+assert ( svNot.starts_with(sv0));
+assert (!svNot.starts_with(sv1));
+assert (!svNot.starts_with(sv2));
+assert (!svNot.starts_with(sv3));
+assert (!svNot.starts_with(sv4));
+assert (!svNot.starts_with(sv5));
+assert ( svNot.starts_with(svNot));
+}
+
+#if TEST_STD_VER > 11
+{
+typedef std::basic_string_view SV;
+constexpr const char *s = "abcde";
+constexpr SV  sv0 {};
+constexpr SV  sv1 { s, 1 };
+constexpr SV  sv2 { s, 2 };
+constexpr SV  sv3 { s, 3 };
+constexpr SV  sv4 { s, 4 };
+constexpr SV  sv5 { s, 5 };
+constexpr SV  svNot {"def", 3 };
+
+static_assert ( sv0.starts_with(sv0), "" );
+static_assert (!sv0.starts_with(sv1), "" );
+
+static_assert ( sv1.starts_with(sv0), "" );
+static_assert ( sv1.starts_with(sv1), "" );
+static_assert (!sv1.starts_with(sv2), "" );
+static_assert (!sv1.starts_with(sv3), "" );
+static_assert (!sv1.starts_with(sv4), "" );
+static_assert (!sv1.starts_with(sv5), "" );
+static_assert (!sv1.starts_with(svNot), "" );
+
+static_assert ( sv2.starts_with(sv0), "" );
+static_assert ( sv2.starts_with(sv1), "" );
+static_assert ( sv2.starts_with(sv2), "" );
+static_assert (!sv2.starts_with(sv3), "" );
+static_assert (!sv2.starts_with(sv4), "" );
+static_assert (!sv2.starts_with(sv5), "" );
+static_assert (!sv2.starts_with(svNot), "" );
+
+static_assert ( svNot.starts_with(sv0), "" );
+static_assert (!svNot.starts_with(sv1), "" );
+static_assert (!svNot.starts_with(sv2), "" );
+static_assert (!svNot.starts_with(sv3), "" );
+static_assert (!svNot.starts_with(sv4), "" );
+static_assert (!svNot.starts_with(sv5), "" );
+static_assert ( svNot.starts_with(svNot), "" );
+}
+#endif
+}
Index: test/std/strings/string.view/string.view.template/starts_with.ptr.pass.cpp

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added inline comments.



Comment at: docs/TaggedAddressSanitizerDesign.rst:22
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.

davidxl wrote:
> What is the overhead of redzones compared with shadow memory?
depends.
shadow is 1/9 of all memory.
redzones largely depend on the allocation patterns. 
If most heap allocations are big, the combined redzones are tiny, and vise 
versa 




Comment at: docs/TaggedAddressSanitizerDesign.rst:49
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the

davidxl wrote:
> a real runtime call or it will be lowered into inline sequence?
at least in the initial implementation -- yes. 
Since this is aarc64-specific, the call/ret should be relatively cheap. 
But of course nothing prevents us from inlining if we see a need for it. 


Repository:
  rC Clang

https://reviews.llvm.org/D40568



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

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: docs/TaggedAddressSanitizerDesign.rst:22
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.

What is the overhead of redzones compared with shadow memory?



Comment at: docs/TaggedAddressSanitizerDesign.rst:49
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the

a real runtime call or it will be lowered into inline sequence?


Repository:
  rC Clang

https://reviews.llvm.org/D40568



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124675.
juliehockett added a comment.

Fixing typo


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or method 

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

I defer to tra on this.


https://reviews.llvm.org/D40453



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124669.
juliehockett marked 4 inline comments as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or 

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D40527#937282, @nik wrote:

> Could you elaborate on "the client will be able to use it to generate a 
> reproducer for the crash"? Having the json file, what would I need to do in 
> order to reproduce the crash?


Sure. I would like to teach clang how to interpret this file so that it can 
generate a reproducer for a crash (similar to a `-gen-reproducer`) option in 
clang. This way external IDE clients could leverage clang's reproducer 
infrastructure to generate reproducers for clang crashes. The external client 
will be responsible for determining when a crash happens and what files have to 
be passed to clang.


Repository:
  rC Clang

https://reviews.llvm.org/D40527



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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/string:309
+   bool ends_with(charT c) const noexcept;
+   bool ends_with(const charT* s) const;
+

rsmith wrote:
> The indentation here seems off. Should these have a `// C++2a` comment?
I may have let a tab sneak in here. I'll be sure to de-tab before committing.



Comment at: include/string_view:577
+   bool starts_with(basic_string_view __s) const _NOEXCEPT
+   { return size() >= __s.size() && compare(0, __s.size(), __s) == 0; }
+

rsmith wrote:
> Is this a conforming implementation? The `size()` check isn't part of the 
> specification, and `compare` could run arbitrary user-supplied code, so the 
> absence of a call to it seems observable.
> 
> Don't get me wrong: I think this check is the right thing to do, and it only 
> makes a difference if `traits::compare` has observable side-effects. But we 
> should file a bug against the standard to get this check added there.
The code as specified in the paper has a bug in it ;-)
I'll be filing an LWG issue once we have a draft standard with these bits in it.


https://reviews.llvm.org/D40586



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

[PATCH] D40258: [CMake] Support side-by-side checkouts in multi-stage build

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319267: [CMake] Support side-by-side checkouts in 
multi-stage build (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D40258?vs=123606=124665#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40258

Files:
  cfe/trunk/CMakeLists.txt


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -603,7 +603,9 @@
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
 CMAKE_MAKE_PROGRAM
-CMAKE_OSX_ARCHITECTURES)
+CMAKE_OSX_ARCHITECTURES
+LLVM_ENABLE_PROJECTS
+LLVM_ENABLE_RUNTIMES)
 
   # We don't need to depend on compiler-rt if we're building instrumented
   # because the next stage will use the same compiler used to build this stage.


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -603,7 +603,9 @@
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
 CMAKE_MAKE_PROGRAM
-CMAKE_OSX_ARCHITECTURES)
+CMAKE_OSX_ARCHITECTURES
+LLVM_ENABLE_PROJECTS
+LLVM_ENABLE_RUNTIMES)
 
   # We don't need to depend on compiler-rt if we're building instrumented
   # because the next stage will use the same compiler used to build this stage.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319267 - [CMake] Support side-by-side checkouts in multi-stage build

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Nov 28 16:34:46 2017
New Revision: 319267

URL: http://llvm.org/viewvc/llvm-project?rev=319267=rev
Log:
[CMake] Support side-by-side checkouts in multi-stage build

Passthrough LLVM_ENABLE_{PROJECTS,RUNTIMES} to followup stages to
support the side-by-side checkouts (aka monorepo layout).

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

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=319267=319266=319267=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Tue Nov 28 16:34:46 2017
@@ -603,7 +603,9 @@ if (CLANG_ENABLE_BOOTSTRAP)
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
 CMAKE_MAKE_PROGRAM
-CMAKE_OSX_ARCHITECTURES)
+CMAKE_OSX_ARCHITECTURES
+LLVM_ENABLE_PROJECTS
+LLVM_ENABLE_RUNTIMES)
 
   # We don't need to depend on compiler-rt if we're building instrumented
   # because the next stage will use the same compiler used to build this stage.


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

[PATCH] D40257: [CMake] Use LIST_SEPARATOR rather than escaping in ExternalProject_Add

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319264: [CMake] Use LIST_SEPARATOR rather than escaping in 
ExternalProject_Add (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D40257?vs=123710=124663#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40257

Files:
  cfe/trunk/CMakeLists.txt


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -653,7 +653,7 @@
   foreach(variableName ${variableNames})
 if(variableName MATCHES "^BOOTSTRAP_")
   string(SUBSTRING ${variableName} 10 -1 varName)
-  string(REPLACE ";" "\;" value "${${variableName}}")
+  string(REPLACE ";" "," value "${${variableName}}")
   list(APPEND PASSTHROUGH_VARIABLES
 -D${varName}=${value})
 endif()
@@ -669,7 +669,7 @@
   if("${${variableName}}" STREQUAL "")
 set(value "")
   else()
-string(REPLACE ";" "\;" value ${${variableName}})
+string(REPLACE ";" "," value "${${variableName}}")
   endif()
   list(APPEND PASSTHROUGH_VARIABLES
 -D${variableName}=${value})
@@ -697,6 +697,7 @@
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+LIST_SEPARATOR ,
 )
 
   # exclude really-install from main target


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -653,7 +653,7 @@
   foreach(variableName ${variableNames})
 if(variableName MATCHES "^BOOTSTRAP_")
   string(SUBSTRING ${variableName} 10 -1 varName)
-  string(REPLACE ";" "\;" value "${${variableName}}")
+  string(REPLACE ";" "," value "${${variableName}}")
   list(APPEND PASSTHROUGH_VARIABLES
 -D${varName}=${value})
 endif()
@@ -669,7 +669,7 @@
   if("${${variableName}}" STREQUAL "")
 set(value "")
   else()
-string(REPLACE ";" "\;" value ${${variableName}})
+string(REPLACE ";" "," value "${${variableName}}")
   endif()
   list(APPEND PASSTHROUGH_VARIABLES
 -D${variableName}=${value})
@@ -697,6 +697,7 @@
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+LIST_SEPARATOR ,
 )
 
   # exclude really-install from main target
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319264 - [CMake] Use LIST_SEPARATOR rather than escaping in ExternalProject_Add

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Nov 28 16:23:10 2017
New Revision: 319264

URL: http://llvm.org/viewvc/llvm-project?rev=319264=rev
Log:
[CMake] Use LIST_SEPARATOR rather than escaping in ExternalProject_Add

Escaping ; in list arguments passed to ExternalProject_Add doesn't seem
to be working in newer versions of CMake (see
https://public.kitware.com/Bug/view.php?id=16137 for more details). Use
a custom LIST_SEPARATOR instead which is the officially supported way.

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

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=319264=319263=319264=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Tue Nov 28 16:23:10 2017
@@ -653,7 +653,7 @@ if (CLANG_ENABLE_BOOTSTRAP)
   foreach(variableName ${variableNames})
 if(variableName MATCHES "^BOOTSTRAP_")
   string(SUBSTRING ${variableName} 10 -1 varName)
-  string(REPLACE ";" "\;" value "${${variableName}}")
+  string(REPLACE ";" "," value "${${variableName}}")
   list(APPEND PASSTHROUGH_VARIABLES
 -D${varName}=${value})
 endif()
@@ -669,7 +669,7 @@ if (CLANG_ENABLE_BOOTSTRAP)
   if("${${variableName}}" STREQUAL "")
 set(value "")
   else()
-string(REPLACE ";" "\;" value ${${variableName}})
+string(REPLACE ";" "," value "${${variableName}}")
   endif()
   list(APPEND PASSTHROUGH_VARIABLES
 -D${variableName}=${value})
@@ -697,6 +697,7 @@ if (CLANG_ENABLE_BOOTSTRAP)
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+LIST_SEPARATOR ,
 )
 
   # exclude really-install from main target


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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

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

rsmith added inline comments.



Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 

erichkeane wrote:
> rsmith wrote:
> > rsmith wrote:
> > > erichkeane wrote:
> > > > rsmith wrote:
> > > > > This seems to be backwards?
> > > > > 
> > > > > Also, I'm not sure whether this is correct. In the strange case where 
> > > > > `Width` is not a multiple of `Align` (because we don't round up the 
> > > > > width), there is no padding. We should only set `HasPadding` to 
> > > > > `true` in the `alignTo` case below.
> > > > I think you're right about it being backwards.
> > > > 
> > > > However, doesn't the struct with a single Ptr and either 1 or 3 Ints 
> > > > have tail padding as well?  I do believe you're right about the alignTo 
> > > > situation below, but only if Width changes, right?
> > > I think the idea is that a struct with one pointer and an odd number of 
> > > ints, on 32-bit Windows, will have no padding per se, but its size will 
> > > simply not be a multiple of its alignment. (So struct layout can put 
> > > things in the four bytes after it, but will insert padding before it to 
> > > place it at an 8 byte boundary.)
> > See example here: https://godbolt.org/g/Nr8C2L
> Well... oh dear.  
> 
> That would mean that: 
>   auto p = ::N;
>   static_assert(!has_unique_object_representations_v); // not 
> unique, since it has padding, right?
>  
>   struct S {
>  decltype(p) s;
>   };
>   static_assert(!has_unique_object_representations_v); // also not unique, 
> since 's' has padding.
> 
>   struct S2 {
>  decltype(p) s;
>  int a;
>   };
>   static_assert(has_unique_object_representations_v); // Actually unique 
> again, since the padding is filled.
> 
> 
> Do I have this right?
The first `static_assert` looks wrong. Should be:

```
static_assert(has_unique_object_representations_v); // unique, has 
no padding
```

Note that `sizeof(decltype(p))` is the size without any padding. The type does 
not have padding itself, but might induce lead padding (due to alignment) and 
tail padding (due to size not being divisible by alignment) in objects it's 
placed within. But I think your current approach will get this all right, so 
long as `MPI.HasPadding` is only set to `true` in the cases where there 
actually is padding in the `MPI.Width` bytes of the representation (which only 
happens when `Width` is rounded up for alignment).


https://reviews.llvm.org/D39347



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:76
+  for (const auto  : Node->bases()) {
+const RecordType *Ty = I.getType()->getAs();
+assert(Ty && "RecordType of base class is unknown");

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:98
+void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {
+  if (const CXXRecordDecl *D =
+Result.Nodes.getNodeAs("decl")) {

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:104
+for (const auto  : D->bases()) {
+  const RecordType *Ty = I.getType()->getAs();
+  assert(Ty && "RecordType of base class is unknown");

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:106
+  assert(Ty && "RecordType of base class is unknown");
+  CXXRecordDecl *Base = 
cast(Ty->getDecl()->getDefinition());
+  if (!MultipleInheritanceCheck::isInterface(Base))

Could be const auto *.


https://reviews.llvm.org/D40580



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124662.
juliehockett marked 6 inline comments as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2183-2184
+  for (const auto *Field : RD->fields()) {
+if (!Context.hasUniqueObjectRepresentations(Field->getType()))
+  return false;
+int64_t FieldSizeInBits =

rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > erichkeane wrote:
> > > > rsmith wrote:
> > > > > What should happen for fields of reference type?
> > > > References are not trivially copyable, so they will prevent the struct 
> > > > from having a unique object representation.
> > > That sounds like the wrong behavior to me. If two structs have references 
> > > that bind to the same object, then they have the same object 
> > > representation, so the struct does have unique object representations.
> > I didn't think of it that way... I WILL note that GCC rejects references in 
> > their implementation, but that could be a bug on their part.
> The wording you quote below is defective in this regard (it doesn't discuss 
> what "same value" means for reference members) -- that's at least partly my 
> wording, sorry about that! But at least my intent when we were working on the 
> rules was that "same value" would have the obvious structurally-recursive 
> meaning, which for references means we're considering whether they have the 
> same referent.
> 
> So I think references, like pointers, should always be considered to have 
> unique object representations when considered as members of objects of class 
> type. (But `__has_unique_object_representations(T&)` should still return 
> `false` because `T&` is not a trivially-copyable type, even though a class 
> containing a `T&` might be.)
That makes sense to me, I can change that.



Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 

rsmith wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > rsmith wrote:
> > > > This seems to be backwards?
> > > > 
> > > > Also, I'm not sure whether this is correct. In the strange case where 
> > > > `Width` is not a multiple of `Align` (because we don't round up the 
> > > > width), there is no padding. We should only set `HasPadding` to `true` 
> > > > in the `alignTo` case below.
> > > I think you're right about it being backwards.
> > > 
> > > However, doesn't the struct with a single Ptr and either 1 or 3 Ints have 
> > > tail padding as well?  I do believe you're right about the alignTo 
> > > situation below, but only if Width changes, right?
> > I think the idea is that a struct with one pointer and an odd number of 
> > ints, on 32-bit Windows, will have no padding per se, but its size will 
> > simply not be a multiple of its alignment. (So struct layout can put things 
> > in the four bytes after it, but will insert padding before it to place it 
> > at an 8 byte boundary.)
> See example here: https://godbolt.org/g/Nr8C2L
Well... oh dear.  

That would mean that: 
  auto p = ::N;
  static_assert(!has_unique_object_representations_v); // not 
unique, since it has padding, right?
 
  struct S {
 decltype(p) s;
  };
  static_assert(!has_unique_object_representations_v); // also not unique, 
since 's' has padding.

  struct S2 {
 decltype(p) s;
 int a;
  };
  static_assert(has_unique_object_representations_v); // Actually unique 
again, since the padding is filled.


Do I have this right?


https://reviews.llvm.org/D39347



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21
+
+  AST_MATCHER(CXXRecordDecl, hasDefinition) {
+if (!Node.hasDefinition())

Please reduce indentation level.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:58
+  // Interfaces should have exclusively pure methods.
+  for (auto method : Node->methods()) {
+if (method->isUserProvided() && !method->isPure()) {

const auto?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:78
+assert(Ty && "RecordType of base class is unknown");
+CXXRecordDecl *Base = cast(Ty->getDecl()->getDefinition());
+if (!MultipleInheritanceCheck::isInterface(Base)) {

Could be (const?) auto *, since you spell type in cast. Same in other places.



Comment at: docs/ReleaseNotes.rst:60
 
+- New `fuchsia-multiple-inheritance
+  
`_
 check

Please move it to previous fuchsia check in alphabetical order.



Comment at: docs/ReleaseNotes.rst:65
+
 - New `objc-avoid-spinlock
   `_ 
check

Please move this in alphabetical order after renamed checks.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:8
+
+For example, Declaring a class that inherits from multiple concrete classes is
+disallowed:

Declaring -> declaring 


https://reviews.llvm.org/D40580



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

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

rsmith added inline comments.



Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 

rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > This seems to be backwards?
> > > 
> > > Also, I'm not sure whether this is correct. In the strange case where 
> > > `Width` is not a multiple of `Align` (because we don't round up the 
> > > width), there is no padding. We should only set `HasPadding` to `true` in 
> > > the `alignTo` case below.
> > I think you're right about it being backwards.
> > 
> > However, doesn't the struct with a single Ptr and either 1 or 3 Ints have 
> > tail padding as well?  I do believe you're right about the alignTo 
> > situation below, but only if Width changes, right?
> I think the idea is that a struct with one pointer and an odd number of ints, 
> on 32-bit Windows, will have no padding per se, but its size will simply not 
> be a multiple of its alignment. (So struct layout can put things in the four 
> bytes after it, but will insert padding before it to place it at an 8 byte 
> boundary.)
See example here: https://godbolt.org/g/Nr8C2L


https://reviews.llvm.org/D39347



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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Sorry for the extra bits.




Comment at: include/string:1365
 static _LIBCPP_INLINE_VISIBILITY
 size_type __recommend(size_type __s) _NOEXCEPT
+{

This is a leftover change. Disregard this bit.



Comment at: 
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp:26
+
 template 
 void

This one too.



Comment at: 
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp:51
 #endif
+{
+// https://bugs.llvm.org/show_bug.cgi?id=31454

This one three.


https://reviews.llvm.org/D40586



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

[PATCH] D40586: Implement p0457r2 - String Prefix and Suffix Checking

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists created this revision.

This adds six calls each to `basic_string` and `basic_string_view`:

- starts_with (3 variants)
- ends_with (3 variants)

This is a C++2a feature


https://reviews.llvm.org/D40586

Files:
  include/string
  include/string_view
  test/std/strings/basic.string/string.ends_with/ends_with.char.pass.cpp
  test/std/strings/basic.string/string.ends_with/ends_with.ptr.pass.cpp
  test/std/strings/basic.string/string.ends_with/ends_with.string_view.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
  test/std/strings/basic.string/string.starts_with/starts_with.char.pass.cpp
  test/std/strings/basic.string/string.starts_with/starts_with.ptr.pass.cpp
  
test/std/strings/basic.string/string.starts_with/starts_with.string_view.pass.cpp
  test/std/strings/string.view/string.view.template/ends_with.char.pass.cpp
  test/std/strings/string.view/string.view.template/ends_with.ptr.pass.cpp
  
test/std/strings/string.view/string.view.template/ends_with.string_view.pass.cpp
  test/std/strings/string.view/string.view.template/starts_with.char.pass.cpp
  test/std/strings/string.view/string.view.template/starts_with.ptr.pass.cpp
  
test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp

Index: test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
===
--- test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
+++ test/std/strings/string.view/string.view.template/starts_with.string_view.pass.cpp
@@ -0,0 +1,104 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// 
+
+//   constexpr bool starts_with(string_view x) const noexcept;
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "constexpr_char_traits.hpp"
+
+int main()
+{
+{
+typedef std::string_view SV;
+const char *s = "abcde";
+SV  sv0 {};
+SV  sv1 { s, 1 };
+SV  sv2 { s, 2 };
+SV  sv3 { s, 3 };
+SV  sv4 { s, 4 };
+SV  sv5 { s, 5 };
+SV  svNot {"def", 3 };
+
+ASSERT_NOEXCEPT(sv0.starts_with(sv0));
+
+assert ( sv0.starts_with(sv0));
+assert (!sv0.starts_with(sv1));
+
+assert ( sv1.starts_with(sv0));
+assert ( sv1.starts_with(sv1));
+assert (!sv1.starts_with(sv2));
+assert (!sv1.starts_with(sv3));
+assert (!sv1.starts_with(sv4));
+assert (!sv1.starts_with(sv5));
+assert (!sv1.starts_with(svNot));
+
+assert ( sv2.starts_with(sv0));
+assert ( sv2.starts_with(sv1));
+assert ( sv2.starts_with(sv2));
+assert (!sv2.starts_with(sv3));
+assert (!sv2.starts_with(sv4));
+assert (!sv2.starts_with(sv5));
+assert (!sv2.starts_with(svNot));
+
+assert ( svNot.starts_with(sv0));
+assert (!svNot.starts_with(sv1));
+assert (!svNot.starts_with(sv2));
+assert (!svNot.starts_with(sv3));
+assert (!svNot.starts_with(sv4));
+assert (!svNot.starts_with(sv5));
+assert ( svNot.starts_with(svNot));
+}
+
+#if TEST_STD_VER > 11
+{
+typedef std::basic_string_view SV;
+constexpr const char *s = "abcde";
+constexpr SV  sv0 {};
+constexpr SV  sv1 { s, 1 };
+constexpr SV  sv2 { s, 2 };
+constexpr SV  sv3 { s, 3 };
+constexpr SV  sv4 { s, 4 };
+constexpr SV  sv5 { s, 5 };
+constexpr SV  svNot {"def", 3 };
+
+static_assert ( sv0.starts_with(sv0), "" );
+static_assert (!sv0.starts_with(sv1), "" );
+
+static_assert ( sv1.starts_with(sv0), "" );
+static_assert ( sv1.starts_with(sv1), "" );
+static_assert (!sv1.starts_with(sv2), "" );
+static_assert (!sv1.starts_with(sv3), "" );
+static_assert (!sv1.starts_with(sv4), "" );
+static_assert (!sv1.starts_with(sv5), "" );
+static_assert (!sv1.starts_with(svNot), "" );
+
+static_assert ( sv2.starts_with(sv0), "" );
+static_assert ( sv2.starts_with(sv1), "" );
+static_assert ( sv2.starts_with(sv2), "" );
+static_assert (!sv2.starts_with(sv3), "" );
+static_assert (!sv2.starts_with(sv4), "" );
+static_assert (!sv2.starts_with(sv5), "" );
+static_assert (!sv2.starts_with(svNot), "" );
+
+static_assert ( svNot.starts_with(sv0), "" );
+static_assert (!svNot.starts_with(sv1), "" );
+static_assert (!svNot.starts_with(sv2), "" );
+static_assert (!svNot.starts_with(sv3), "" );
+static_assert (!svNot.starts_with(sv4), "" );
+static_assert (!svNot.starts_with(sv5), "" );
+static_assert ( svNot.starts_with(svNot), "" );
+}
+#endif
+}
Index: 

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich updated this revision to Diff 124646.
jakehehrlich added a comment.

When I changed the test to add "_cc1" I got rid of the temporary file. Well for 
some reason on windows we're still getting different output so we need the 
temporary file to debug things.


Repository:
  rL LLVM

https://reviews.llvm.org/D39627

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGVTT.cpp
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGen/push-hidden-visibility-subclass.cpp

Index: test/CodeGen/push-hidden-visibility-subclass.cpp
===
--- /dev/null
+++ test/CodeGen/push-hidden-visibility-subclass.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -emit-llvm %s -o %t
+// RUN: FileCheck --input-file=%t %s
+
+#pragma GCC visibility push(hidden)
+
+struct Base {
+  virtual ~Base() = default;
+  virtual void* Alloc() = 0;
+};
+
+class Child : public Base {
+public:
+  Child() = default;
+  void* Alloc();
+};
+
+void test() {
+  Child x;
+}
+
+// CHECK: @_ZTV5Child = external hidden unnamed_addr constant
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1527,7 +1527,7 @@
 VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
 
   // Set the right visibility.
-  CGM.setGlobalVisibility(VTable, RD);
+  CGM.setGlobalVisibility(VTable, RD, ForDefinition);
 
   // Use pointer alignment for the vtable. Otherwise we would align them based
   // on the size of the initializer which doesn't make sense as only single
@@ -1637,6 +1637,7 @@
   VTable = CGM.CreateOrReplaceCXXRuntimeVariable(
   Name, VTableType, llvm::GlobalValue::ExternalLinkage);
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+  CGM.setGlobalVisibility(VTable, RD, NotForDefinition);
 
   if (RD->hasAttr())
 VTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -710,7 +710,8 @@
   llvm::ConstantInt *getSize(CharUnits numChars);
 
   /// Set the visibility for the given LLVM GlobalValue.
-  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
+  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D,
+   ForDefinition_t IsForDefinition) const;
 
   /// Set the TLS mode for the given LLVM GlobalValue for the thread-local
   /// variable declaration D.
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -669,16 +669,18 @@
 }
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
-const NamedDecl *D) const {
+const NamedDecl *D,
+ForDefinition_t IsForDefinition) const {
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
 GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
 return;
   }
 
   // Set visibility for definitions.
   LinkageInfo LV = D->getLinkageAndVisibility();
-  if (LV.isVisibilityExplicit() || !GV->hasAvailableExternallyLinkage())
+  if (LV.isVisibilityExplicit() ||
+  (IsForDefinition && !GV->hasAvailableExternallyLinkage()))
 GV->setVisibility(GetLLVMVisibility(LV.getVisibility()));
 }
 
@@ -1059,7 +1061,7 @@
 void CodeGenModule::SetCommonAttributes(const Decl *D,
 llvm::GlobalValue *GV) {
   if (const auto *ND = dyn_cast_or_null(D))
-setGlobalVisibility(GV, ND);
+setGlobalVisibility(GV, ND, ForDefinition);
   else
 GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
 
@@ -1115,8 +1117,8 @@
   setNonAliasAttributes(D, F);
 }
 
-static void setLinkageAndVisibilityForGV(llvm::GlobalValue *GV,
- const NamedDecl *ND) {
+static void setLinkageForGV(llvm::GlobalValue *GV,
+const NamedDecl *ND) {
   // Set linkage and visibility in case we never see a definition.
   LinkageInfo LV = ND->getLinkageAndVisibility();
   if (!isExternallyVisible(LV.getLinkage())) {
@@ -1132,10 +1134,6 @@
   // separate linkage types for this.
   GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
 }
-
-// Set visibility on a declaration only if it's explicit.
-if (LV.isVisibilityExplicit())
-  GV->setVisibility(CodeGenModule::GetLLVMVisibility(LV.getVisibility()));
   }
 }
 
@@ -1204,7 +1202,8 @@
   // Only a few attributes are set on declarations; these may later be
   // overridden by a definition.
 
-  setLinkageAndVisibilityForGV(F, FD);
+  setLinkageForGV(F, FD);

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2183-2184
+  for (const auto *Field : RD->fields()) {
+if (!Context.hasUniqueObjectRepresentations(Field->getType()))
+  return false;
+int64_t FieldSizeInBits =

rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > What should happen for fields of reference type?
> > References are not trivially copyable, so they will prevent the struct from 
> > having a unique object representation.
> That sounds like the wrong behavior to me. If two structs have references 
> that bind to the same object, then they have the same object representation, 
> so the struct does have unique object representations.
I didn't think of it that way... I WILL note that GCC rejects references in 
their implementation, but that could be a bug on their part.



Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

rsmith wrote:
> This isn't quite right; you want min(bitfield length, bit size of type) here. 
> Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.
I guess I'm missing something with this comment... why would a bitfield with 
bool b: 8 have padding?

Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an int)?
Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 9, but 
Clang seems to allow 'bool' to have ANY size.  Is that an error itself?

Finally: Should we consider ANY bool to be not a unique object representation?  
It has 1 bit of data, but 8 bits of storage.  GCC's implementation of this 
trait accepts them, but I'm second guessing that at the moment...



Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 

rsmith wrote:
> This seems to be backwards?
> 
> Also, I'm not sure whether this is correct. In the strange case where `Width` 
> is not a multiple of `Align` (because we don't round up the width), there is 
> no padding. We should only set `HasPadding` to `true` in the `alignTo` case 
> below.
I think you're right about it being backwards.

However, doesn't the struct with a single Ptr and either 1 or 3 Ints have tail 
padding as well?  I do believe you're right about the alignTo situation below, 
but only if Width changes, right?


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

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

rsmith added a comment.

Please add tests for the member pointer cases across various different targets.




Comment at: lib/AST/ASTContext.cpp:2183-2184
+  for (const auto *Field : RD->fields()) {
+if (!Context.hasUniqueObjectRepresentations(Field->getType()))
+  return false;
+int64_t FieldSizeInBits =

erichkeane wrote:
> rsmith wrote:
> > What should happen for fields of reference type?
> References are not trivially copyable, so they will prevent the struct from 
> having a unique object representation.
That sounds like the wrong behavior to me. If two structs have references that 
bind to the same object, then they have the same object representation, so the 
struct does have unique object representations.



Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

This isn't quite right; you want min(bitfield length, bit size of type) here. 
Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.



Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 

This seems to be backwards?

Also, I'm not sure whether this is correct. In the strange case where `Width` 
is not a multiple of `Align` (because we don't round up the width), there is no 
padding. We should only set `HasPadding` to `true` in the `alignTo` case below.


https://reviews.llvm.org/D39347



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

LLVM buildmaster is back to work now but two builders remain OFF

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster is back to work.

Two slaves remain off for now as they produce a lot of warnings and their
log files are way too big.

The next builds have full logs available. Hope this helps to fix this issue:

http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/8716
http://lab.llvm.org:8011/builders/clang-with-thin-lto-windows/builds/2645

Thanks

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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a check to the Fuchsia module to warn when a class inheritsfrom multiple 
classes that are not pure virtual.

See https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md for reference.


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, Declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

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



Comment at: docs/TaggedAddressSanitizerDesign.rst:2
+===
+TaggedAddressSanitizer Design Documentation
+===

eugenis wrote:
> I vote for HardwareAssistedAddressSanitizer, or hwasan.
Added hwasan as an alternative. 
I dislike it less then tasan. 
Let's wait for other suggestions before renaming. 



Comment at: docs/TaggedAddressSanitizerDesign.rst:101
+and some overhead due to `N`-aligning all objects.
+
+

eugenis wrote:
> Mention the system calls problem with tagged pointers.
added 
   May require changes in the OS kernels (e.g. Linux seems to dislike
tagged pointers passed from address space).


Repository:
  rC Clang

https://reviews.llvm.org/D40568



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

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc updated this revision to Diff 124637.
kcc added a comment.

addressed comments


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst

Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,122 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** or TASAN
+(alternative: **HardwareAssistedAddressSanitizer** or HWASAN)
+and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` on mismatch.
+The function name encodes the type and the size of access, e.g. `__hwams_load4(void *ptr)`.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * May require changes in the OS kernels (e.g. Linux seems to dislike
+tagged pointers passed from address space).
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
+.. _AddressSanitizer paper: https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
+.. _Address Tagging: 

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 124636.

https://reviews.llvm.org/D40453

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -75,6 +75,11 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro(D.getVFS).IsDebian())
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto  : CudaPathCandidates) {


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -75,6 +75,11 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro(D.getVFS).IsDebian())
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto  : CudaPathCandidates) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40577: Clang support for simd functions

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a reviewer: ABataev.
Hahnfeld added a comment.

This should add or extend a regression test


https://reviews.llvm.org/D40577



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

[PATCH] D40577: Clang support for simd functions

[Matt via Phabricator via cfe-commits]

mmasten added a comment.

This patch is related to revisions https://reviews.llvm.org/D22792 and 
https://reviews.llvm.org/D40575


https://reviews.llvm.org/D40577



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

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:81
+
+if (Distro.IsDebian())
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");

No need for a named temporary. `if (Distro(D.getVFS).IsDebian())` should do.

Also, please add a comment describing why Debian needs a special case here and 
include reference to the bug report which has the details.



https://reviews.llvm.org/D40453



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

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

On a related note please add context to your patch as described here: 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


https://reviews.llvm.org/D40453



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

[PATCH] D40577: Clang support for simd functions

[Matt via Phabricator via cfe-commits]

mmasten created this revision.

This patch adds "vector-variants" function attributes to simd functions.


https://reviews.llvm.org/D40577

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp


Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7565,10 +7565,18 @@
 Masked.push_back('M');
 break;
   }
+
+  std::string Buffer;
+  if (Fn->hasFnAttribute("vector-variants")) {
+llvm::Attribute Attr = Fn->getFnAttribute("vector-variants");
+Buffer = Attr.getValueAsString().str();
+  }
+  llvm::raw_string_ostream Out(Buffer);
+
   for (auto Mask : Masked) {
 for (auto  : ISAData) {
-  SmallString<256> Buffer;
-  llvm::raw_svector_ostream Out(Buffer);
+  if (!Buffer.empty())
+Out << ",";
   Out << "_ZGV" << Data.ISA << Mask;
   if (!VLENVal) {
 Out << llvm::APSInt::getUnsigned(Data.VecRegSize /
@@ -7596,9 +7604,11 @@
   Out << 'a' << ParamAttr.Alignment;
   }
   Out << '_' << Fn->getName();
-  Fn->addFnAttr(Out.str());
+  Out.flush();
 }
   }
+
+  Fn->addFnAttr("vector-variants", Out.str());
 }
 
 void CGOpenMPRuntime::emitDeclareSimdFunction(const FunctionDecl *FD,


Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7565,10 +7565,18 @@
 Masked.push_back('M');
 break;
   }
+
+  std::string Buffer;
+  if (Fn->hasFnAttribute("vector-variants")) {
+llvm::Attribute Attr = Fn->getFnAttribute("vector-variants");
+Buffer = Attr.getValueAsString().str();
+  }
+  llvm::raw_string_ostream Out(Buffer);
+
   for (auto Mask : Masked) {
 for (auto  : ISAData) {
-  SmallString<256> Buffer;
-  llvm::raw_svector_ostream Out(Buffer);
+  if (!Buffer.empty())
+Out << ",";
   Out << "_ZGV" << Data.ISA << Mask;
   if (!VLENVal) {
 Out << llvm::APSInt::getUnsigned(Data.VecRegSize /
@@ -7596,9 +7604,11 @@
   Out << 'a' << ParamAttr.Alignment;
   }
   Out << '_' << Fn->getName();
-  Fn->addFnAttr(Out.str());
+  Out.flush();
 }
   }
+
+  Fn->addFnAttr("vector-variants", Out.str());
 }
 
 void CGOpenMPRuntime::emitDeclareSimdFunction(const FunctionDecl *FD,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38425: [clangd] Document highlights for clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:246
+
+  C.reply(json::ary(Highlights->Value));
+}

malaperle wrote:
> I get a test failure here because there is an assertion that the Expected<> 
> needs to be checked. I can't really think of any failure case right now where 
> we wouldn't just return an empty array of highlights. But I think it's better 
> for consistency and future-proofing to keep the Expected<>.
> I think you can just do like in onRename for now
>   if (!Highlights) {
> C.replyError(ErrorCode::InternalError,
>  llvm::toString(Highlights.takeError()));
> return;
>   }
Just to be clear, the error I see in the output is:
Expected must be checked before access or destruction.
Expected value was in success state. (Note: Expected values in success 
mode must still be checked prior to being destroyed).

It asserts when in ClangdLSPServer::onDocumentHighlight, referencing 
Highlights->Value


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425



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

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 124629.

https://reviews.llvm.org/D40453

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -63,6 +63,8 @@
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
 
+  Distro Distro(D.getVFS());
+
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 CudaPathCandidates.push_back(
 Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ));
@@ -75,6 +77,9 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro.IsDebian())
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto  : CudaPathCandidates) {


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -63,6 +63,8 @@
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
 
+  Distro Distro(D.getVFS());
+
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 CudaPathCandidates.push_back(
 Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ));
@@ -75,6 +77,9 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro.IsDebian())
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto  : CudaPathCandidates) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: docs/TaggedAddressSanitizerDesign.rst:2
+===
+TaggedAddressSanitizer Design Documentation
+===

I vote for HardwareAssistedAddressSanitizer, or hwasan.



Comment at: docs/TaggedAddressSanitizerDesign.rst:24
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as

remove "the"



Comment at: docs/TaggedAddressSanitizerDesign.rst:101
+and some overhead due to `N`-aligning all objects.
+
+

Mention the system calls problem with tagged pointers.


Repository:
  rC Clang

https://reviews.llvm.org/D40568



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

[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D40108#938131, @juliehockett wrote:

> If you could, that'd be great! Thank you!


It'll be good idea it you'll request commit rights, since you have more checks 
in development queue.


https://reviews.llvm.org/D40108



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

r319227 - [OPENMP] Generalize capturing of clauses expressions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Nov 28 13:11:44 2017
New Revision: 319227

URL: http://llvm.org/viewvc/llvm-project?rev=319227=rev
Log:
[OPENMP] Generalize capturing of clauses expressions.

The handling and capturing of the non-constant expressions of some of
the capturable clauses in combined directives is generalized.

Modified:
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp

Modified: cfe/trunk/lib/Basic/OpenMPKinds.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/OpenMPKinds.cpp?rev=319227=319226=319227=diff
==
--- cfe/trunk/lib/Basic/OpenMPKinds.cpp (original)
+++ cfe/trunk/lib/Basic/OpenMPKinds.cpp Tue Nov 28 13:11:44 2017
@@ -892,9 +892,12 @@ void clang::getOpenMPCaptureRegions(
 CaptureRegions.push_back(OMPD_target);
 CaptureRegions.push_back(OMPD_teams);
 break;
+  case OMPD_teams:
   case OMPD_teams_distribute:
+  case OMPD_teams_distribute_simd:
 CaptureRegions.push_back(OMPD_teams);
 break;
+  case OMPD_target:
   case OMPD_target_simd:
 CaptureRegions.push_back(OMPD_target);
 break;
@@ -908,12 +911,16 @@ void clang::getOpenMPCaptureRegions(
 CaptureRegions.push_back(OMPD_target);
 CaptureRegions.push_back(OMPD_parallel);
 break;
+  case OMPD_task:
   case OMPD_target_enter_data:
   case OMPD_target_exit_data:
   case OMPD_target_update:
 CaptureRegions.push_back(OMPD_task);
 break;
-  case OMPD_teams:
+  case OMPD_taskloop:
+  case OMPD_taskloop_simd:
+CaptureRegions.push_back(OMPD_taskloop);
+break;
   case OMPD_simd:
   case OMPD_for:
   case OMPD_for_simd:
@@ -927,18 +934,13 @@ void clang::getOpenMPCaptureRegions(
   case OMPD_ordered:
   case OMPD_atomic:
   case OMPD_target_data:
-  case OMPD_target:
-  case OMPD_task:
-  case OMPD_taskloop:
-  case OMPD_taskloop_simd:
   case OMPD_distribute_simd:
-  case OMPD_teams_distribute_simd:
   case OMPD_teams_distribute_parallel_for_simd:
   case OMPD_target_teams_distribute:
   case OMPD_target_teams_distribute_parallel_for:
   case OMPD_target_teams_distribute_parallel_for_simd:
   case OMPD_target_teams_distribute_simd:
-CaptureRegions.push_back(DKind);
+CaptureRegions.push_back(OMPD_unknown);
 break;
   case OMPD_threadprivate:
   case OMPD_taskyield:

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=319227=319226=319227=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Nov 28 13:11:44 2017
@@ -2394,6 +2394,8 @@ StmtResult Sema::ActOnOpenMPRegionEnd(St
 return StmtError();
   }
 
+  SmallVector CaptureRegions;
+  getOpenMPCaptureRegions(CaptureRegions, DSAStack->getCurrentDirective());
   OMPOrderedClause *OC = nullptr;
   OMPScheduleClause *SC = nullptr;
   SmallVector LCs;
@@ -2422,7 +2424,8 @@ StmtResult Sema::ActOnOpenMPRegionEnd(St
 }
   }
   DSAStack->setForceVarCapturing(/*V=*/false);
-} else if (isParallelOrTaskRegion(DSAStack->getCurrentDirective())) {
+} else if (CaptureRegions.size() > 1 ||
+   CaptureRegions.back() != OMPD_unknown) {
   if (auto *C = OMPClauseWithPreInit::get(Clause))
 PICs.push_back(C);
   if (auto *C = OMPClauseWithPostUpdate::get(Clause)) {
@@ -2470,13 +2473,11 @@ StmtResult Sema::ActOnOpenMPRegionEnd(St
 return StmtError();
   }
   StmtResult SR = S;
-  SmallVector CaptureRegions;
-  getOpenMPCaptureRegions(CaptureRegions, DSAStack->getCurrentDirective());
-  for (auto ThisCaptureRegion : llvm::reverse(CaptureRegions)) {
+  for (OpenMPDirectiveKind ThisCaptureRegion : llvm::reverse(CaptureRegions)) {
 // Mark all variables in private list clauses as used in inner region.
 // Required for proper codegen of combined directives.
 // TODO: add processing for other clauses.
-if (isParallelOrTaskRegion(DSAStack->getCurrentDirective())) {
+if (ThisCaptureRegion != OMPD_unknown) {
   for (auto *C : PICs) {
 OpenMPDirectiveKind CaptureRegion = C->getCaptureRegion();
 // Find the particular capture region for the clause if the
@@ -7533,19 +7534,21 @@ static OpenMPDirectiveKind getOpenMPCapt
 OpenMPDirectiveKind DKind, OpenMPClauseKind CKind,
 OpenMPDirectiveKind NameModifier = OMPD_unknown) {
   OpenMPDirectiveKind CaptureRegion = OMPD_unknown;
-
   switch (CKind) {
   case OMPC_if:
 switch (DKind) {
 case OMPD_target_parallel:
 case OMPD_target_parallel_for:
 case OMPD_target_parallel_for_simd:
+case OMPD_target_teams_distribute_parallel_for:
+case OMPD_target_teams_distribute_parallel_for_simd:
   // If this clause applies to the nested 'parallel' region, capture within
   // the 'target' region, otherwise 

[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit in r319225.


https://reviews.llvm.org/D40108



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

[clang-tools-extra] r319225 - Add a new clang-tidy module for Fuchsia as an umbrella to diagnose issues with the Fuschia and Zircon coding guidelines (https://fuchsia.googlesource.com/zircon/+/master/

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Nov 28 13:09:25 2017
New Revision: 319225

URL: http://llvm.org/viewvc/llvm-project?rev=319225=rev
Log:
Add a new clang-tidy module for Fuchsia as an umbrella to diagnose issues with 
the Fuschia and Zircon coding guidelines 
(https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md). Adds the first 
of such checkers, which detects when default arguments are declared in a 
function declaration or when default arguments are used at call sites.

Patch by Julie Hockett

Added:
clang-tools-extra/trunk/clang-tidy/fuchsia/
clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp
clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.h
clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/fuchsia-default-arguments.rst
clang-tools-extra/trunk/test/clang-tidy/fuchsia-default-arguments.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/index.rst

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=319225=319224=319225=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Tue Nov 28 13:09:25 2017
@@ -31,6 +31,7 @@ add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
+add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)
 add_subdirectory(llvm)

Added: clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt?rev=319225=auto
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt Tue Nov 28 
13:09:25 2017
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyFuchsiaModule
+  DefaultArgumentsCheck.cpp
+  FuchsiaTidyModule.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )

Added: clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp?rev=319225=auto
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp Tue 
Nov 28 13:09:25 2017
@@ -0,0 +1,64 @@
+//===--- DefaultArgumentsCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DefaultArgumentsCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+void DefaultArgumentsCheck::registerMatchers(MatchFinder *Finder) {
+  // Calling a function which uses default arguments is disallowed.
+  Finder->addMatcher(cxxDefaultArgExpr().bind("stmt"), this);
+  // Declaring default parameters is disallowed.
+  Finder->addMatcher(parmVarDecl(hasDefaultArgument()).bind("decl"), this);
+}
+
+void DefaultArgumentsCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *S =
+  Result.Nodes.getNodeAs("stmt")) {
+diag(S->getUsedLocation(),
+ "calling a function that uses a default argument is disallowed");
+diag(S->getParam()->getLocStart(),
+"default parameter was declared here",
+ DiagnosticIDs::Note);
+  } else if (const ParmVarDecl *D =
+  Result.Nodes.getNodeAs("decl")) {
+SourceRange DefaultArgRange = D->getDefaultArgRange();
+
+if (DefaultArgRange.getEnd() != D->getLocEnd()) {
+  return;
+} else if (DefaultArgRange.getBegin().isMacroID()) {
+  diag(D->getLocStart(),
+   "declaring a parameter with a default argument is disallowed");
+} else {
+  SourceLocation StartLocation = D->getName().empty() ?
+  D->getLocStart() : D->getLocation();
+
+  SourceRange RemovalRange(Lexer::getLocForEndOfToken(
+ StartLocation, 0,
+ *Result.SourceManager,
+ 

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

I'm getting "multiple definition" errors on powerpc64le when linking the unit 
tests, for example `XRayBufferQueueTest`:

  src/projects/compiler-rt/lib/xray/xray_utils.cc:34: multiple definition of 
`__xray::retryingWriteAll(int, char*, char*)'
  src/projects/compiler-rt/lib/xray/xray_utils.cc:73: multiple definition of 
`__xray::readValueFromFile(char const*, long long*)'
  src/projects/compiler-rt/lib/xray/xray_utils.cc:95: multiple definition of 
`__xray::getLogFD()'

to name a few of the errors.

(related note: Why aren't the unit tests rebuilt if the 
`libclang_rt.xray-powerpc64le.a` changes?!? After reverting this commit locally 
for testing, the unit test built just fine. After reapplying the test wasn't 
rebuilt and I saw no problems...)


https://reviews.llvm.org/D39114



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

[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

If you could, that'd be great! Thank you!


https://reviews.llvm.org/D40108



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.

Fixes: PR28520

Perform a bounds check on a function's argument list, before accessing any 
index value specified by an 'argument_with_type_tag' attribute.


https://reviews.llvm.org/D40574

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/Sema/error-type-safety.cpp

Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+
+void test_invalid_type_tag_index(int datatype, ...)
+  __attribute__((argument_with_type_tag(test,1,99)));
+
+void test_invalid_arg_index(int datatype, ...)
+  __attribute__((argument_with_type_tag(test,99,1)));
+
+void test_bounds()
+{
+  test_invalid_type_tag_index(0); // expected-error {{type tag index 99 is greater than the number of arguments specified}}
+  test_invalid_arg_index(0); // expected-error {{argument tag index 99 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,11 +12329,20 @@
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_type_tag_out_of_range)
+<< Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+
   bool FoundWrongKind;
   TypeTagData TypeInfo;
   if (!GetMatchingCType(ArgumentKind, TypeTagExpr, Context,
@@ -12346,7 +12355,15 @@
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_arg_tag_out_of_range)
+<< Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).
 if (const ImplicitCastExpr *ICE = dyn_cast(ArgumentExpr))
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,10 @@
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_type_tag_out_of_range : Error<
+  "type tag index %0 is greater than the number of arguments specified">;
+def err_arg_tag_out_of_range : Error<
+  "argument tag index %0 is greater than the number of arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39947: [OpenMP] Stable sort Privates to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319222: [OpenMP] Stable sort Privates to remove 
non-deterministic ordering (authored by mgrang).

Changed prior to commit:
  https://reviews.llvm.org/D39947?vs=122751=124622#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39947

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp


Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4056,9 +4056,9 @@
   return TaskPrivatesMap;
 }
 
-static int array_pod_sort_comparator(const PrivateDataTy *P1,
- const PrivateDataTy *P2) {
-  return P1->first < P2->first ? 1 : (P2->first < P1->first ? -1 : 0);
+static bool stable_sort_comparator(const PrivateDataTy P1,
+   const PrivateDataTy P2) {
+  return P1.first > P2.first;
 }
 
 /// Emit initialization for private variables in task-based directives.
@@ -4286,8 +4286,7 @@
  /*PrivateElemInit=*/nullptr)));
 ++I;
   }
-  llvm::array_pod_sort(Privates.begin(), Privates.end(),
-   array_pod_sort_comparator);
+  std::stable_sort(Privates.begin(), Privates.end(), stable_sort_comparator);
   auto KmpInt32Ty = C.getIntTypeForBitwidth(/*DestWidth=*/32, /*Signed=*/1);
   // Build type kmp_routine_entry_t (if not built yet).
   emitKmpRoutineEntryT(KmpInt32Ty);


Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4056,9 +4056,9 @@
   return TaskPrivatesMap;
 }
 
-static int array_pod_sort_comparator(const PrivateDataTy *P1,
- const PrivateDataTy *P2) {
-  return P1->first < P2->first ? 1 : (P2->first < P1->first ? -1 : 0);
+static bool stable_sort_comparator(const PrivateDataTy P1,
+   const PrivateDataTy P2) {
+  return P1.first > P2.first;
 }
 
 /// Emit initialization for private variables in task-based directives.
@@ -4286,8 +4286,7 @@
  /*PrivateElemInit=*/nullptr)));
 ++I;
   }
-  llvm::array_pod_sort(Privates.begin(), Privates.end(),
-   array_pod_sort_comparator);
+  std::stable_sort(Privates.begin(), Privates.end(), stable_sort_comparator);
   auto KmpInt32Ty = C.getIntTypeForBitwidth(/*DestWidth=*/32, /*Signed=*/1);
   // Build type kmp_routine_entry_t (if not built yet).
   emitKmpRoutineEntryT(KmpInt32Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40299: [Complex] Don't use __div?c3 when building with fast-math.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added reviewers: arphaman, GorNishanov, hfinkel, fhahn.
fhahn added a comment.

Looks good to me, with some nits. However it seems that we do not use the fast 
math flags anywhere else in Clang codegen, so it would be good to clarify if it 
is OK to use it here.




Comment at: lib/CodeGen/CGExprComplex.cpp:768
+
 // If we have a complex operand on the RHS, we delegate to a libcall to
 // handle all of the complexities and minimize underflow/overflow cases.

Maybe we should update the comment that we do a similar simplification for 
floats with fast math enabled as for integers?



Comment at: lib/CodeGen/CGExprComplex.cpp:773
 // supported imaginary types in addition to complex types.
-if (RHSi) {
+if (RHSi && !FMF.isFast()) {
   BinOpInfo LibCallOp = Op;

Would the following structure be slightly easier to read?

if (RHSi) {
  if (FMF.isFast()) { simplify } else {libcall}
}



Comment at: lib/CodeGen/CGExprComplex.cpp:800
+  // (a+ib) / (c+id) = ((ac+bd)/(cc+dd)) + i((bc-ad)/(cc+dd))
+  llvm::Value *AC = Builder.CreateFMul(LHSr, RHSr); // a*c
+  llvm::Value *BD = Builder.CreateFMul(LHSi, RHSi); // b*d

This is basically the same as the simplification for integers, unfortunate that 
we have to duplicate it (because of different instructions).


https://reviews.llvm.org/D40299



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

[PATCH] D40572: [OpenMP] Make test robust against quotation, NFC.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.

This is needed to not break with an upcoming change in LLVM to use
dollar signs (which need to be quoted) instead of dots for
generating unique names.


https://reviews.llvm.org/D40572

Files:
  test/OpenMP/nvptx_parallel_codegen.cpp

Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -92,20 +92,20 @@
   //
   // CHECK: [[EXEC_PARALLEL]]
   // CHECK: [[WF1:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1:@.+]]_wrapper to i8*)
+  // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1:@.+]]_wrapper{{"?}} to i8*)
   // CHECK: br i1 [[WM1]], label {{%?}}[[EXEC_PFN1:.+]], label {{%?}}[[CHECK_NEXT1:.+]]
   //
   // CHECK: [[EXEC_PFN1]]
-  // CHECK: call void [[PARALLEL_FN1]]_wrapper(
+  // CHECK: call void [[PARALLEL_FN1]]_wrapper{{"?}}(
   // CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
   //
   // CHECK: [[CHECK_NEXT1]]
   // CHECK: [[WF2:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2:@.+]]_wrapper to i8*)
+  // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2:@.+]]_wrapper{{"?}} to i8*)
   // CHECK: br i1 [[WM2]], label {{%?}}[[EXEC_PFN2:.+]], label {{%?}}[[CHECK_NEXT2:.+]]
   //
   // CHECK: [[EXEC_PFN2]]
-  // CHECK: call void [[PARALLEL_FN2]]_wrapper(
+  // CHECK: call void [[PARALLEL_FN2]]_wrapper{{"?}}(
   // CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
   //
   // CHECK: [[CHECK_NEXT2]]
@@ -152,13 +152,13 @@
   // CHECK-DAG: [[MWS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
   // CHECK: [[MTMP1:%.+]] = sub i32 [[MNTH]], [[MWS]]
   // CHECK: call void @__kmpc_kernel_init(i32 [[MTMP1]]
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1]]_wrapper to i8*),
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1]]_wrapper{{"?}} to i8*),
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @__kmpc_serialized_parallel(
-  // CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]](
+  // CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]]{{"?}}(
   // CHECK: call void @__kmpc_end_serialized_parallel(
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2]]_wrapper to i8*),
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2]]_wrapper{{"?}} to i8*),
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK-64-DAG: load i32, i32* [[REF_A]]
@@ -173,17 +173,17 @@
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK-DAG: define internal void [[PARALLEL_FN1]](
+  // CHECK-DAG: define internal void [[PARALLEL_FN1]]{{"?}}(
   // CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
   // CHECK: store i[[SZ]] 42, i[[SZ]]* %a,
   // CHECK: ret void
 
-  // CHECK-DAG: define internal void [[PARALLEL_FN3]](
+  // CHECK-DAG: define internal void [[PARALLEL_FN3]]{{"?}}(
   // CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
   // CHECK: store i[[SZ]] 43, i[[SZ]]* %a,
   // CHECK: ret void
 
-  // CHECK-DAG: define internal void [[PARALLEL_FN2]](
+  // CHECK-DAG: define internal void [[PARALLEL_FN2]]{{"?}}(
   // CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
   // CHECK: store i[[SZ]] 44, i[[SZ]]* %a,
   // CHECK: ret void
@@ -217,11 +217,11 @@
   //
   // CHECK: [[EXEC_PARALLEL]]
   // CHECK: [[WF:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM:%.+]] = icmp eq i8* [[WF]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4:@.+]]_wrapper to i8*)
+  // CHECK: [[WM:%.+]] = icmp eq i8* [[WF]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4:@.+]]_wrapper{{"?}} to i8*)
   // CHECK: br i1 [[WM]], label {{%?}}[[EXEC_PFN:.+]], label {{%?}}[[CHECK_NEXT:.+]]
   //
   // CHECK: [[EXEC_PFN]]
-  // CHECK: call void [[PARALLEL_FN4]]_wrapper(
+  // CHECK: call void [[PARALLEL_FN4]]_wrapper{{"?}}(
   // CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
   //
   // CHECK: [[CHECK_NEXT]]
@@ -283,14 +283,14 @@
   // CHECK: br i1 [[CMP]], label {{%?}}[[IF_THEN:.+]], label {{%?}}[[IF_ELSE:.+]]
   //
   // CHECK: [[IF_THEN]]
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4]]_wrapper to i8*),
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4]]_wrapper{{"?}} to i8*),
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: br label {{%?}}[[IF_END:.+]]
   //
   // CHECK: [[IF_ELSE]]
   // CHECK: call void @__kmpc_serialized_parallel(
-  // CHECK: {{call|invoke}} void [[PARALLEL_FN4]](

[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

My skepticism about the raw_ostream is not about the design of having a custom 
raw_ostream subclass, it's that that subclass could conceivably be re-used by 
some other client.  It feels like it belongs as an internal hack in Clang 
absent some real evidence that someone else would use it.




Comment at: lib/AST/TypePrinter.cpp:1532-1534
+namespace {
+template
+void printTo(raw_ostream , ArrayRef Args, const PrintingPolicy ,

sepavloff wrote:
> rnk wrote:
> > `static` is nicer than a short anonymous namespace.
> Yes, but this is function template. It won't create symbol in object file. 
> Actually anonymous namespace has no effect here, it is only a documentation 
> hint.
Nonetheless, we generally prefer to use 'static' on internal functions, even 
function templates, instead of putting them in anonymous namespaces.


https://reviews.llvm.org/D40508



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

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

[Reid Kleckner via Phabricator via cfe-commits]

rnk requested changes to this revision.
rnk added inline comments.
This revision now requires changes to proceed.



Comment at: lib/Driver/ToolChain.cpp:450-458
+  const llvm::Target *TT = llvm::TargetRegistry::lookupTarget(TS, Error);
+  if (!TT)
+return llvm::ExceptionHandling::None;
+  std::unique_ptr MRI(TT->createMCRegInfo(TS));
+  if (!MRI)
+return llvm::ExceptionHandling::None;
+  std::unique_ptr MAI(TT->createMCAsmInfo(*MRI, TS));

I think this is problematic because Clang is supposed to be able to generate 
LLVM IR for targets that are not registered. With this change, we have silent 
behavior difference between clang with LLVM backend and clang without LLVM 
backends.

Building clang without a single backend is pretty silly, but our test suite 
frequently generates IR for targets that are not compiled in. Some of the tests 
you've written will probably fail on buildbots configured this way.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673



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

[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2188
+   !Context.getTargetInfo().isVLASupported() &&
+   shouldDiagnoseTargetSupportFromOpenMP()) {
+  // Some targets don't support VLAs.

tra wrote:
> rjmccall wrote:
> > tra wrote:
> > > tra wrote:
> > > > rjmccall wrote:
> > > > > Please write this check so that it trips in an "ordinary" build on a 
> > > > > target that just happens to not support VLAs, something like:
> > > > > 
> > > > >   else if (!Context.getTargetInfo().isVLASupported() && 
> > > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > > > 
> > > > > If you want to include the explicit OpenMP check there, it would need 
> > > > > to be:
> > > > > 
> > > > >   else if (!Context.getTargetInfo().isVLASupported() && 
> > > > > (!getLangOpts().OpenMP || shouldDiagnoseTargetSupportFromOpenMP()))
> > > > > 
> > > > > but I think the first looks better.
> > > > > 
> > > > > The CUDA and OpenMP paths here seem to be trying to achieve analogous 
> > > > > things; it's unfortunate that we can't find a way to unify their 
> > > > > approaches, even if we'd eventually want to use different diagnostic 
> > > > > text.  I imagine that the target-environment language restrictions 
> > > > > are basically the same, since they arise for the same fundamental 
> > > > > reasons, so all the places using CUDADiagIfDeviceCode are likely to 
> > > > > have a check for shouldDiagnoseTargetSupportFromOpenMP() and 
> > > > > vice-versa.
> > > > The problem is that in CUDA we can't just do 
> > > > ```
> > > > if (!Context.getTargetInfo().isVLASupported() && 
> > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > >Diag(Loc, diag::err_vla_unsupported);
> > > > ```
> > > > 
> > > > In some situations diag messages will only be emitted if we attempt to 
> > > > generate unsupported code on device side.
> > > > Consider 
> > > > ```
> > > > __host__ __device__ void foo(int n) {
> > > >   int vla[n];
> > > > }
> > > > ```
> > > > 
> > > > When Sema sees this code during compilation, it can not tell whether 
> > > > there is an error. Calling foo from the host code is perfectly valid. 
> > > > Calling it from device code is not. `CUDADiagIfDeviceCode` creates 
> > > > 'postponed' diagnostics which only gets emitted if we ever need to 
> > > > generate code for the function on device.
> > > > 
> > > > So, while CUDA and OpenMP do similar things, they are not quite the 
> > > > same.  If your goal to generalize CUDA and OpenMP handling, then it 
> > > > would have to be folded into diagnostic-emitting itself and we'll need 
> > > > an analog of CUDADiagIfDeviceCode which can handle both OpenMP and 
> > > > CUDA. 
> > > > E.g. something like this:
> > > > ```
> > > > ??? DiagIfDeviceCode(???) {
> > > >if (OpenCL || (OpenMP && shouldDiagnoseTargetSupportFromOpenMP()))
> > > >Diag(...);
> > > >else if (CUDA)
> > > >CUDADiagIfDeviceCode()
> > > > } 
> > > > 
> > > > ...
> > > > 
> > > > if (!Context.getTargetInfo().isVLASupported()) 
> > > >DiagIfDeviceCode();
> > > > 
> > > > ```
> > > > 
> > > > Would that work for you?
> > > There's another issue with this approach -- diagnostics itself. Each 
> > > dialect has its own. Specifically CUDA diags have details that are 
> > > relevant only to CUDA. I suspect OpenMP has something specific as well. 
> > > If we insist emitting only one kind of error for particular case across 
> > > all dialects, we'll have to stick to bare bones "feature X is not 
> > > supported" which will not have sufficient details to explain why the 
> > > error was triggered in CUDA.
> > > 
> > > IMO dialect-specific handling of cuda errors in this case is the lesser 
> > > evil. 
> > > 
> > > I'll update the patch to handle non-cuda cases the way  you suggested.
> > If there really is interesting language-specific information to provide in 
> > a diagnostic, I agree that it's hard to avoid having different code for 
> > different targets.  On the other hand, the CUDA-specific information in 
> > this specific diagnostic seems unnecessary — does the user really care 
> > about whether the function was  '__device__' vs. '__host__ __device__'? in 
> > fact, isn't the latter a bit misleading? — and I feel like a generic 
> > "cannot use variable-length arrays when compiling for device 'nvptx64'" 
> > would be perfectly satisfactory in both CUDA and OpenMP, and that's 
> > probably true for almost all of these diagnostics.
> > 
> > On a completely different note, I do want to point out that the only thing 
> > you actually *need* to ban here is declaring a local variable of VLA type.  
> > There's no reason at all to ban VLA types in general; they just compile to 
> > extra arithmetic.
> Agreed. While host/device attributes are part of a function signature in 
> CUDA, in this case only 'device' part makes the difference and therefore 
> common-style reporting the way you suggest would be 

[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.

[John McCall via Phabricator via cfe-commits]

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

Okay, seems reasonable enough.


Repository:
  rC Clang

https://reviews.llvm.org/D40569



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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

Thanks for tackling this! There is one major comment inline (you are generating 
an extra node that you shouldn't, which is causing the analyzer to explore code 
it shouldn't) and a suggestion for simpler diagnostic text.




Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

"Unary operator" is probably not something we should expect users to know 
about. How about just "The expression is an uninitialized value. The computed 
value will also be garbage."




Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1046
 if (V2_untested.isUnknownOrUndef()) {
   Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+

Instead of generating a node node here, you'll want to generate a new state:
```
state = state->BindExpr(U, LCtx, V2_untested);
```
and use that state in the call to evalStore().

Otherwise, you'll end up exploring from both the new generated node and from *I.

Here is a test that demonstrates why this is a problem. You should add it to 
your tests.

```
void foo() {
  int x;

  x++; // expected-warning {{The expression of the unary operator is an 
uninitialized value. The computed value will also be garbage}}

  clang_analyzer_warnIfReached(); // no-warning

```
The undefined assignment checker generates what we call "fatal" errors. These 
errors are so bad that it decides not to explore further on that path to report 
additional errors. This means that the analyzer should treat any code that is 
dominated by such an error as not reachable. You'll need to add the 
`debug.ExprInspection` checker to your test RUN line and a prototype for 
clang_analyzer_warnIfReached() for this test to to work.




Comment at: test/Analysis/malloc-plist.c:2
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -analyzer-config path-diagnostics-alternate=false -o %t 
%s
 // RUN: FileCheck -input-file %t %s

xazax.hun wrote:
> In case you willing to replace the plist based test with 
> `-analyzer-output=text` based, it would be awesome, though not required for 
> this patch to be accepted.
Let's hold off on replacing the plist test with text. We'll need to make sure 
we're still testing plist emission separately.



Comment at: test/Analysis/malloc-plist.c:13
 int *p = malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is 
an uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

Once you change the core modeling to not generate an extra node, you'll want to 
initialize `*p` to `0` or something to preserve the intent of this test. For 
this test it is important that the increment not stop further exploration of 
the path.



Comment at: test/Analysis/malloc-plist.c:111
 p = (int*)malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is an 
uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

Same point applies here.



Comment at: test/Analysis/objc-for.m:131
 int i;
 int j;
 for (NSString *a in A) {

You should just initialize `j` in order to preserve the test's intent. (And 
similarly for the other new warnings in this file.



Comment at: test/Analysis/uninit-const.c:127
+  int x; // expected-note 5 {{'x' declared without an initial value}}
+  x++; // expected-warning {{The expression of the unary operator is an 
uninitialized value. The computed value will also be garbage}}
+   // expected-note@-1 {{The expression of the unary operator is an 
uninitialized value. The computed value will also be garbage}}

You'll need to split this test into multiple functions once you stop generating 
the extra node.



Comment at: test/Analysis/uninit-const.cpp:11
 
+// https://bugs.llvm.org/show_bug.cgi?id=35419
+void f11(void) {

I don't think this test is needed. The logic for C vs. C++ is shared for your 
new functionality, so I think it is sufficient to just test the C case.


Repository:
  rL LLVM

https://reviews.llvm.org/D40463



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

[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion

[Tony Jiang via Phabricator via cfe-commits]

jtony added inline comments.



Comment at: test/Driver/ppc-features.cpp:140
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-htm -### -o %t.o 
2>&1 | FileCheck -check-prefix=CHECK-NOHTM %s
 // CHECK-NOHTM: "-target-feature" "-htm"

We probably need to add test  for mno_save_toc_indirect also once you add it 
according to Hal's suggestion.


https://reviews.llvm.org/D39376



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

[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.

[Jacob Young via Phabricator via cfe-commits]

jacobly created this revision.

Forcing the 4 byte alignment caused an issue in my out of tree backend that 
doesn't even have a 4 byte aligned stack.  Using the default target-specific 
alignment for i32 seems more reasonable.


Repository:
  rC Clang

https://reviews.llvm.org/D40569

Files:
  lib/CodeGen/CGCleanup.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h


Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -433,7 +433,7 @@
   };
 
   /// i32s containing the indexes of the cleanup destinations.
-  llvm::AllocaInst *NormalCleanupDest;
+  Address NormalCleanupDest;
 
   unsigned NextCleanupDestIndex;
 
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -70,7 +70,7 @@
   IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
   SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
   BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
-  NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+  NormalCleanupDest(Address::invalid()), NextCleanupDestIndex(1),
   FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
   EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
   DisableDebugInfo(false), DidCallStackSave(false), 
IndirectBranch(nullptr),
@@ -433,10 +433,11 @@
   // if compiled with no optimizations. We do it for coroutine as the lifetime
   // of CleanupDestSlot alloca make correct coroutine frame building very
   // difficult.
-  if (NormalCleanupDest && isCoroutine()) {
+  if (NormalCleanupDest.isValid() && isCoroutine()) {
 llvm::DominatorTree DT(*CurFn);
-llvm::PromoteMemToReg(NormalCleanupDest, DT);
-NormalCleanupDest = nullptr;
+llvm::PromoteMemToReg(
+cast(NormalCleanupDest.getPointer()), DT);
+NormalCleanupDest = Address::invalid();
   }
 }
 
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -833,7 +833,7 @@
 if (NormalCleanupDestSlot->hasOneUse()) {
   NormalCleanupDestSlot->user_back()->eraseFromParent();
   NormalCleanupDestSlot->eraseFromParent();
-  NormalCleanupDest = nullptr;
+  NormalCleanupDest = Address::invalid();
 }
 
 llvm::BasicBlock *BranchAfter = Scope.getBranchAfterBlock(0);
@@ -1250,10 +1250,10 @@
 }
 
 Address CodeGenFunction::getNormalCleanupDestSlot() {
-  if (!NormalCleanupDest)
+  if (!NormalCleanupDest.isValid())
 NormalCleanupDest =
-  CreateTempAlloca(Builder.getInt32Ty(), "cleanup.dest.slot");
-  return Address(NormalCleanupDest, CharUnits::fromQuantity(4));
+  CreateDefaultAlignTempAlloca(Builder.getInt32Ty(), "cleanup.dest.slot");
+  return NormalCleanupDest;
 }
 
 /// Emits all the code to cause the given temporary to be cleaned up.


Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -433,7 +433,7 @@
   };
 
   /// i32s containing the indexes of the cleanup destinations.
-  llvm::AllocaInst *NormalCleanupDest;
+  Address NormalCleanupDest;
 
   unsigned NextCleanupDestIndex;
 
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -70,7 +70,7 @@
   IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
   SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
   BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
-  NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+  NormalCleanupDest(Address::invalid()), NextCleanupDestIndex(1),
   FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
   EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
   DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
@@ -433,10 +433,11 @@
   // if compiled with no optimizations. We do it for coroutine as the lifetime
   // of CleanupDestSlot alloca make correct coroutine frame building very
   // difficult.
-  if (NormalCleanupDest && isCoroutine()) {
+  if (NormalCleanupDest.isValid() && isCoroutine()) {
 llvm::DominatorTree DT(*CurFn);
-llvm::PromoteMemToReg(NormalCleanupDest, DT);
-NormalCleanupDest = nullptr;
+llvm::PromoteMemToReg(
+cast(NormalCleanupDest.getPointer()), DT);
+NormalCleanupDest = Address::invalid();
   }
 }
 
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -833,7 +833,7 @@
   

LLVM buildmaster will be restarted in few minutes

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be restarted in few minutes.
Thank you for understanding.

Thanks

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

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb added a comment.

Ping @compnerd, @sdardis


https://reviews.llvm.org/D38110



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

[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Re: "I tried to write a test for this, and convinced myself that while +1 is 
correct, it is currently impossible to change behavior based on the missing 
+1.":
In order to have different outcome based on the start column, you could use 
tabs. Consider the content `"aaa\tb"` with 4 spaces of tabstop. This takes up 5 
columns (say, under the column limit) if it starts at column 0, and 8 columns 
(say, over the column limit) if it starts at column 1.


https://reviews.llvm.org/D40310



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124609.
erichkeane added a comment.

Add has padding to CXXABI getMemberPointerWidthAndAlign to correct padding 
behavior here.


https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext ) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not unique.
-  if ((*this)->isFunctionType())
-return false;
-
-  // All integrals and 

[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/BreakableToken.cpp:178
   Split Split) const {
   // Example: consider the content
   // lala  lala

Offtopic: Should add a FIXME that this doesn't really work in the presence of 
tabs.



Comment at: lib/Format/BreakableToken.h:154
   /// \p Split has been compressed into a single space.
   unsigned getLineLengthAfterCompression(unsigned RemainingTokenColumns,
  Split Split) const;

nit: the comment doesn't make sense anymore.



Comment at: lib/Format/ContinuationIndenter.cpp:1625
+  if (ExcessCharactersPenalty < NewBreakPenalty) {
+ContinueOnLine = true;
+  }

nit: no braces around single-statement if body



Comment at: lib/Format/ContinuationIndenter.cpp:1707
+  RemainingTokenColumns = Token->getRemainingLength(
+  NextLineIndex, TailOffset, ContentStartColumn);
+  Reflow = true;

When we're reflowing we replace the reflow split with a space, so IMO this 
should be:
```
RemainingTokenColumns = Token->getRemainingLength(
  NextLineIndex, TailOffset, ContentStartColumn + 1);
```



Comment at: lib/Format/ContinuationIndenter.cpp:1709
+  Reflow = true;
+  if (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+DEBUG(llvm::dbgs() << "Over limit after reflow, need: "

IMO this should be `ContentStartColumn + RemainingTokenColumns + 1`, accounting 
for the space that replaces the reflow split.



Comment at: lib/Format/ContinuationIndenter.cpp:1721
+Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+ContentStartColumn, CommentPragmasRegex);
+if (Split.first == StringRef::npos) {

Looks like `ContentStartColumn` is consistently used as the start column of the 
reflown content on the next line.
I suggest to `++ContentStartColumn` at the beginning of the body of this if 
statement (and adjust below appropriately).


https://reviews.llvm.org/D40310



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

[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319201: [CUDA] Report "unsupported VLA" errors only on 
device side. (authored by tra).

Changed prior to commit:
  https://reviews.llvm.org/D40275?vs=123831=124607#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40275

Files:
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
  cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
  cfe/trunk/test/SemaCUDA/vla.cu


Index: cfe/trunk/test/SemaCUDA/vla.cu
===
--- cfe/trunk/test/SemaCUDA/vla.cu
+++ cfe/trunk/test/SemaCUDA/vla.cu
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
===
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only 
-verify %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
===
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only 
-verify %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -2180,14 +2180,17 @@
 Diag(Loc, diag::err_opencl_vla);
 return QualType();
   }
-  // CUDA device code doesn't support VLAs.
-  if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
-  // Some targets don't support VLAs.
-  if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported() &&
-  shouldDiagnoseTargetSupportFromOpenMP()) {
-Diag(Loc, diag::err_vla_unsupported);
-return QualType();
+
+  if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
+if (getLangOpts().CUDA) {
+  // CUDA device code doesn't support VLAs.
+  CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
+} else if (!getLangOpts().OpenMP ||
+   shouldDiagnoseTargetSupportFromOpenMP()) {
+  // Some targets don't support VLAs.
+  Diag(Loc, diag::err_vla_unsupported);
+  return QualType();
+}
   }
 
   // If this is not C99, extwarn about VLA's and C99 array size modifiers.


Index: cfe/trunk/test/SemaCUDA/vla.cu
===
--- cfe/trunk/test/SemaCUDA/vla.cu
+++ cfe/trunk/test/SemaCUDA/vla.cu
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
===
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only -verify %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
===
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only -verify %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -2180,14 +2180,17 @@
 Diag(Loc, diag::err_opencl_vla);
 return QualType();
   }
-  // CUDA device code doesn't support VLAs.
-  if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
-  // Some targets don't support VLAs.
-  

r319201 - [CUDA] Report "unsupported VLA" errors only on device side.

[Artem Belevich via cfe-commits]

Author: tra
Date: Tue Nov 28 10:51:42 2017
New Revision: 319201

URL: http://llvm.org/viewvc/llvm-project?rev=319201=rev
Log:
[CUDA] Report "unsupported VLA" errors only on device side.

This fixes erroneously reported CUDA compilation errors
in host-side code during device-side compilation.

I've also restricted OpenMP-specific checks to trigger only
if we're compiling with OpenMP enabled.

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

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
cfe/trunk/test/SemaCUDA/vla.cu

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=319201=319200=319201=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Nov 28 10:51:42 2017
@@ -2180,14 +2180,17 @@ QualType Sema::BuildArrayType(QualType T
 Diag(Loc, diag::err_opencl_vla);
 return QualType();
   }
-  // CUDA device code doesn't support VLAs.
-  if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
-  // Some targets don't support VLAs.
-  if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported() &&
-  shouldDiagnoseTargetSupportFromOpenMP()) {
-Diag(Loc, diag::err_vla_unsupported);
-return QualType();
+
+  if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
+if (getLangOpts().CUDA) {
+  // CUDA device code doesn't support VLAs.
+  CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
+} else if (!getLangOpts().OpenMP ||
+   shouldDiagnoseTargetSupportFromOpenMP()) {
+  // Some targets don't support VLAs.
+  Diag(Loc, diag::err_vla_unsupported);
+  return QualType();
+}
   }
 
   // If this is not C99, extwarn about VLA's and C99 array size modifiers.

Modified: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu?rev=319201=319200=319201=diff
==
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu (original)
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu Tue Nov 28 10:51:42 
2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only 
-verify %s
 
 #include "Inputs/cuda.h"
 

Modified: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu?rev=319201=319200=319201=diff
==
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu (original)
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu Tue Nov 28 
10:51:42 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only 
-verify %s
 
 #include "Inputs/cuda.h"
 

Modified: cfe/trunk/test/SemaCUDA/vla.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/vla.cu?rev=319201=319200=319201=diff
==
--- cfe/trunk/test/SemaCUDA/vla.cu (original)
+++ cfe/trunk/test/SemaCUDA/vla.cu Tue Nov 28 10:51:42 2017
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
 
 #include "Inputs/cuda.h"
 


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

[PATCH] D39730: Enabling constructor code completion

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I'll take a look, but my guess is that you might have to use scoping information


https://reviews.llvm.org/D39730



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

[PATCH] D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Ping * 2.


https://reviews.llvm.org/D40044



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

[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2188
+   !Context.getTargetInfo().isVLASupported() &&
+   shouldDiagnoseTargetSupportFromOpenMP()) {
+  // Some targets don't support VLAs.

rjmccall wrote:
> tra wrote:
> > tra wrote:
> > > rjmccall wrote:
> > > > Please write this check so that it trips in an "ordinary" build on a 
> > > > target that just happens to not support VLAs, something like:
> > > > 
> > > >   else if (!Context.getTargetInfo().isVLASupported() && 
> > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > > 
> > > > If you want to include the explicit OpenMP check there, it would need 
> > > > to be:
> > > > 
> > > >   else if (!Context.getTargetInfo().isVLASupported() && 
> > > > (!getLangOpts().OpenMP || shouldDiagnoseTargetSupportFromOpenMP()))
> > > > 
> > > > but I think the first looks better.
> > > > 
> > > > The CUDA and OpenMP paths here seem to be trying to achieve analogous 
> > > > things; it's unfortunate that we can't find a way to unify their 
> > > > approaches, even if we'd eventually want to use different diagnostic 
> > > > text.  I imagine that the target-environment language restrictions are 
> > > > basically the same, since they arise for the same fundamental reasons, 
> > > > so all the places using CUDADiagIfDeviceCode are likely to have a check 
> > > > for shouldDiagnoseTargetSupportFromOpenMP() and vice-versa.
> > > The problem is that in CUDA we can't just do 
> > > ```
> > > if (!Context.getTargetInfo().isVLASupported() && 
> > > shouldDiagnoseTargetSupportFromOpenMP())
> > >Diag(Loc, diag::err_vla_unsupported);
> > > ```
> > > 
> > > In some situations diag messages will only be emitted if we attempt to 
> > > generate unsupported code on device side.
> > > Consider 
> > > ```
> > > __host__ __device__ void foo(int n) {
> > >   int vla[n];
> > > }
> > > ```
> > > 
> > > When Sema sees this code during compilation, it can not tell whether 
> > > there is an error. Calling foo from the host code is perfectly valid. 
> > > Calling it from device code is not. `CUDADiagIfDeviceCode` creates 
> > > 'postponed' diagnostics which only gets emitted if we ever need to 
> > > generate code for the function on device.
> > > 
> > > So, while CUDA and OpenMP do similar things, they are not quite the same. 
> > >  If your goal to generalize CUDA and OpenMP handling, then it would have 
> > > to be folded into diagnostic-emitting itself and we'll need an analog of 
> > > CUDADiagIfDeviceCode which can handle both OpenMP and CUDA. 
> > > E.g. something like this:
> > > ```
> > > ??? DiagIfDeviceCode(???) {
> > >if (OpenCL || (OpenMP && shouldDiagnoseTargetSupportFromOpenMP()))
> > >Diag(...);
> > >else if (CUDA)
> > >CUDADiagIfDeviceCode()
> > > } 
> > > 
> > > ...
> > > 
> > > if (!Context.getTargetInfo().isVLASupported()) 
> > >DiagIfDeviceCode();
> > > 
> > > ```
> > > 
> > > Would that work for you?
> > There's another issue with this approach -- diagnostics itself. Each 
> > dialect has its own. Specifically CUDA diags have details that are relevant 
> > only to CUDA. I suspect OpenMP has something specific as well. If we insist 
> > emitting only one kind of error for particular case across all dialects, 
> > we'll have to stick to bare bones "feature X is not supported" which will 
> > not have sufficient details to explain why the error was triggered in CUDA.
> > 
> > IMO dialect-specific handling of cuda errors in this case is the lesser 
> > evil. 
> > 
> > I'll update the patch to handle non-cuda cases the way  you suggested.
> If there really is interesting language-specific information to provide in a 
> diagnostic, I agree that it's hard to avoid having different code for 
> different targets.  On the other hand, the CUDA-specific information in this 
> specific diagnostic seems unnecessary — does the user really care about 
> whether the function was  '__device__' vs. '__host__ __device__'? in fact, 
> isn't the latter a bit misleading? — and I feel like a generic "cannot use 
> variable-length arrays when compiling for device 'nvptx64'" would be 
> perfectly satisfactory in both CUDA and OpenMP, and that's probably true for 
> almost all of these diagnostics.
> 
> On a completely different note, I do want to point out that the only thing 
> you actually *need* to ban here is declaring a local variable of VLA type.  
> There's no reason at all to ban VLA types in general; they just compile to 
> extra arithmetic.
Agreed. While host/device attributes are part of a function signature in CUDA, 
in this case only 'device' part makes the difference and therefore common-style 
reporting the way you suggest would be sufficient to indicate the error in the 
device-side code.

As for the VLA types, clang can currently compile code with VLA arguments: 
https://godbolt.org/g/43hVu9
Clang explicitly does not support GCC's VLA-in-structure extension. 
Are 

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc updated this revision to Diff 124604.
kcc added a comment.

minor edit (explained shadow)


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst

Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,118 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` on mismatch.
+The function name encodes the type and the size of access, e.g. `__hwams_load4(void *ptr)`.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
+.. _AddressSanitizer paper: https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
+.. _Address Tagging: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D40508: Replace long type names in IR with hashes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

https://reviews.llvm.org/D40567 presents a patch that adds template argument 
names to class template specializations. It also describes the use case in 
which type names are used to identify type across different TUs.
Adding template parameters obviously increase memory consumption. This patch 
provides a way to reduce it.
Note that this issue arises only if source is compiled to bitcode (.ll or .bc). 
If compilation is made to machine representation (.s or .o), IR type name are 
not needed.


https://reviews.llvm.org/D40508



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

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc created this revision.
Herald added subscribers: cfe-commits, fedor.sergeev.

preliminary design document for a hardware-assisted memory safety (HWAMS) tool, 
similar to AddressSanitizer
The name TaggedAddressSanitizer and the rest of the document, are early draft, 
suggestions are welcome.

The code will follow shortly.


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst
  docs/index.rst

Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -84,6 +84,7 @@
PTHInternals
PCHInternals
ItaniumMangleAbiTags
+   TaggedAddressSanitizerDesign
 
 
 Indices and tables
Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,118 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag,
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` on mismatch.
+The function name encodes the type and the size of access, e.g. `__hwams_load4(void *ptr)`.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: 

[PATCH] D40567: Always show template parameters in IR type names

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
Herald added subscribers: JDevlieghere, eraman.

If a module contains opaque type and another one contains definition
of equivalent type in sense of C++, `llvm-link` merges these types.
In this procedure it can rely only on type name. An issue arises if
the type is a class template specialization. See the example.

File 1

  template struct ABC;
  ABC *var_1;

File 2

  template struct ABC {
T *f;
  };
  extern ABC var_1;
  ABC var_2;

llvm-link produces module:

  %struct.ABC = type { i32** }
  @var_1 = global %struct.ABC* null, align 8
  @var_2 = global %struct.ABC zeroinitializer, align 8

Incomplete type `ABC` from the first module becomes `ABC`. It
happens because structure types obtained from template specialization
share the same type name and differ from each other only by version
suffixes, in the example the types are named as `ABC` and `ABC.0`.
`llvm-link` cannot correctly merge these types.

This change enables template arguments in class template specializations.
Clang already prints them in class context, now they will appear in the
class name itself.


https://reviews.llvm.org/D40567

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/apple-kext-indirect-call.cpp
  test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  test/CodeGenCXX/captured-statements.cpp
  test/CodeGenCXX/const-init-cxx11.cpp
  test/CodeGenCXX/constructor-init.cpp
  test/CodeGenCXX/ctor-dtor-alias.cpp
  test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  test/CodeGenCXX/debug-info-template.cpp
  test/CodeGenCXX/dllexport-pr26549.cpp
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/dllimport.cpp
  test/CodeGenCXX/float128-declarations.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenCXX/mangle-abi-tag.cpp
  test/CodeGenCXX/mangle-ms-templates-memptrs-2.cpp
  test/CodeGenCXX/microsoft-abi-extern-template.cpp
  test/CodeGenCXX/microsoft-abi-vftables.cpp
  test/CodeGenCXX/ms-property.cpp
  test/CodeGenCXX/noinline-template.cpp
  test/CodeGenCXX/pr29160.cpp
  test/CodeGenCXX/static-init-3.cpp
  test/CodeGenCXX/template-anonymous-types.cpp
  test/CodeGenCXX/template-instantiation.cpp
  test/CodeGenCXX/template-linkage.cpp
  test/CodeGenCXX/value-init.cpp
  test/CodeGenCXX/virtual-base-destructor-call.cpp
  test/CodeGenCoroutines/coro-await.cpp
  test/CodeGenObjCXX/arc-cxx11-init-list.mm
  test/CodeGenObjCXX/property-lvalue-capture.mm
  test/OpenMP/declare_reduction_codegen.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp

Index: test/OpenMP/target_teams_codegen.cpp
===
--- test/OpenMP/target_teams_codegen.cpp
+++ test/OpenMP/target_teams_codegen.cpp
@@ -303,7 +303,7 @@
   // CHECK-NEXT:  br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
 
   // CHECK:   [[FAIL]]
-  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"{{[^,]+}})
   // CHECK-NEXT:  br label %[[END]]
   // CHECK:   [[END]]
   #pragma omp target teams if(target: n>20)
Index: test/OpenMP/target_simd_codegen.cpp
===
--- test/OpenMP/target_simd_codegen.cpp
+++ test/OpenMP/target_simd_codegen.cpp
@@ -292,7 +292,7 @@
   // CHECK-NEXT:  br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
 
   // CHECK:   [[FAIL]]
-  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"*{{[^,]+}})
   // CHECK-NEXT:  br label %[[END]]
   // CHECK:   [[END]]
   #pragma omp target simd if(target: n>20)
Index: test/OpenMP/target_parallel_codegen.cpp
===
--- test/OpenMP/target_parallel_codegen.cpp
+++ test/OpenMP/target_parallel_codegen.cpp
@@ -279,7 +279,7 @@
   // CHECK-NEXT:  br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
 
   // CHECK:   [[FAIL]]
-  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+  // CHECK:   call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT{{[^,]+}})
   // CHECK-NEXT:  br label %[[END]]
   // CHECK:   [[END]]
   #pragma omp target parallel 

[PATCH] D40228: [Target] Make a copy of TargetOptions feature list before sorting during CodeGen

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319195: [Target] Make a copy of TargetOptions feature list 
before sorting during CodeGen (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D40228?vs=123862=124599#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40228

Files:
  cfe/trunk/lib/CodeGen/CGCall.cpp


Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1877,13 +1877,13 @@
 // we have a decl for the function and it has a target attribute then
 // parse that and add it to the feature set.
 StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
 const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
 if (FD && FD->hasAttr()) {
   llvm::StringMap FeatureMap;
   getFunctionFeatureMap(FeatureMap, FD);
 
   // Produce the canonical string for this set of features.
-  std::vector Features;
   for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
  ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@
   if (ParsedAttr.Architecture != "" &&
   getTarget().isValidCPUName(ParsedAttr.Architecture))
 TargetCPU = ParsedAttr.Architecture;
-  if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
 } else {
   // Otherwise just add the existing target cpu and target features to the
   // function.
-  std::vector  = 
getTarget().getTargetOpts().Features;
-  if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
+  Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+  FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+  std::sort(Features.begin(), Features.end());
+  FuncAttrs.addAttribute(
+  "target-features",
+  llvm::join(Features, ","));
 }
   }
 


Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1877,13 +1877,13 @@
 // we have a decl for the function and it has a target attribute then
 // parse that and add it to the feature set.
 StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
 const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
 if (FD && FD->hasAttr()) {
   llvm::StringMap FeatureMap;
   getFunctionFeatureMap(FeatureMap, FD);
 
   // Produce the canonical string for this set of features.
-  std::vector Features;
   for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
  ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@
   if (ParsedAttr.Architecture != "" &&
   getTarget().isValidCPUName(ParsedAttr.Architecture))
 TargetCPU = ParsedAttr.Architecture;
-  if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
 } else {
   // Otherwise just add the existing target cpu and target features to the
   // function.
-  std::vector  = getTarget().getTargetOpts().Features;
-  if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
+  Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+  FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+  std::sort(Features.begin(), Features.end());
+  FuncAttrs.addAttribute(
+  "target-features",
+  llvm::join(Features, ","));
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for the nit fix. If you'd like me to commit this on your behalf, just 
let me know.


https://reviews.llvm.org/D40108



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

r319195 - [Target] Make a copy of TargetOptions feature list before sorting during CodeGen

[Craig Topper via cfe-commits]

Author: ctopper
Date: Tue Nov 28 10:00:32 2017
New Revision: 319195

URL: http://llvm.org/viewvc/llvm-project?rev=319195=rev
Log:
[Target] Make a copy of TargetOptions feature list before sorting during CodeGen

Currently CodeGen is calling std::sort on the features vector in TargetOptions 
for every function, but I don't think CodeGen should be modifying TargetOptions.

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

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=319195=319194=319195=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Nov 28 10:00:32 2017
@@ -1877,13 +1877,13 @@ void CodeGenModule::ConstructAttributeLi
 // we have a decl for the function and it has a target attribute then
 // parse that and add it to the feature set.
 StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
 const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
 if (FD && FD->hasAttr()) {
   llvm::StringMap FeatureMap;
   getFunctionFeatureMap(FeatureMap, FD);
 
   // Produce the canonical string for this set of features.
-  std::vector Features;
   for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
  ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@ void CodeGenModule::ConstructAttributeLi
   if (ParsedAttr.Architecture != "" &&
   getTarget().isValidCPUName(ParsedAttr.Architecture))
 TargetCPU = ParsedAttr.Architecture;
-  if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
 } else {
   // Otherwise just add the existing target cpu and target features to the
   // function.
-  std::vector  = 
getTarget().getTargetOpts().Features;
-  if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
-  if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
-  }
+  Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+  FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+  std::sort(Features.begin(), Features.end());
+  FuncAttrs.addAttribute(
+  "target-features",
+  llvm::join(Features, ","));
 }
   }
 


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