[PATCH] D25157: [compiler-rt] [cmake] Respect COMPILER_RT_BUILD_* for libs, headers and tests

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

mgorny updated this revision to Diff 93347.
mgorny added a comment.

Needed to rebase again.


https://reviews.llvm.org/D25157

Files:
  cmake/config-ix.cmake
  include/CMakeLists.txt
  lib/CMakeLists.txt
  test/sanitizer_common/CMakeLists.txt

Index: test/sanitizer_common/CMakeLists.txt
===
--- test/sanitizer_common/CMakeLists.txt
+++ test/sanitizer_common/CMakeLists.txt
@@ -4,13 +4,20 @@
 set(SANITIZER_COMMON_TESTSUITES)
 
 set(SUPPORTED_TOOLS)
-if(CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux|FreeBSD" AND NOT ANDROID)
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux|FreeBSD" AND NOT ANDROID AND
+   COMPILER_RT_HAS_ASAN)
   list(APPEND SUPPORTED_TOOLS asan)
 endif()
 if(CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT ANDROID)
-  list(APPEND SUPPORTED_TOOLS tsan)
-  list(APPEND SUPPORTED_TOOLS msan)
-  list(APPEND SUPPORTED_TOOLS lsan)
+  if(COMPILER_RT_HAS_TSAN)
+list(APPEND SUPPORTED_TOOLS tsan)
+  endif()
+  if(COMPILER_RT_HAS_MSAN)
+list(APPEND SUPPORTED_TOOLS msan)
+  endif()
+  if(COMPILER_RT_HAS_LSAN)
+list(APPEND SUPPORTED_TOOLS lsan)
+  endif()
 endif()
 
 # Create a separate config for each tool we support.
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -8,8 +8,7 @@
 # sanitizers or xray (or both).
 #
 #TODO: Refactor sanitizer_common into smaller pieces (e.g. flag parsing, utils).
-if (COMPILER_RT_HAS_SANITIZER_COMMON AND
-(COMPILER_RT_BUILD_SANITIZERS OR COMPILER_RT_BUILD_XRAY))
+if (COMPILER_RT_HAS_SANITIZER_COMMON)
   add_subdirectory(sanitizer_common)
 endif()
 
@@ -36,27 +35,27 @@
   endif()
 endfunction()
 
-if(COMPILER_RT_BUILD_SANITIZERS)
-  compiler_rt_build_runtime(interception)
+# the following set is conditional to COMPILER_RT_BUILD_SANITIZERS
+# (via COMPILER_RT_HAS_* in config-ix.cmake)
+compiler_rt_build_runtime(interception)
 
-  if(COMPILER_RT_HAS_SANITIZER_COMMON)
-add_subdirectory(stats)
-add_subdirectory(lsan)
-add_subdirectory(ubsan)
-  endif()
+if(COMPILER_RT_BUILD_SANITIZERS AND COMPILER_RT_HAS_SANITIZER_COMMON)
+  add_subdirectory(stats)
+  add_subdirectory(lsan)
+  add_subdirectory(ubsan)
+endif()
 
-  compiler_rt_build_sanitizer(asan)
-  compiler_rt_build_sanitizer(dfsan)
-  compiler_rt_build_sanitizer(msan)
-  compiler_rt_build_sanitizer(tsan tsan/dd)
-  compiler_rt_build_sanitizer(safestack)
-  compiler_rt_build_sanitizer(cfi)
-  compiler_rt_build_sanitizer(esan)
-  compiler_rt_build_sanitizer(scudo)
+compiler_rt_build_sanitizer(asan)
+compiler_rt_build_sanitizer(dfsan)
+compiler_rt_build_sanitizer(msan)
+compiler_rt_build_sanitizer(tsan tsan/dd)
+compiler_rt_build_sanitizer(safestack)
+compiler_rt_build_sanitizer(cfi)
+compiler_rt_build_sanitizer(esan)
+compiler_rt_build_sanitizer(scudo)
 
-  compiler_rt_build_runtime(profile)
-endif()
+compiler_rt_build_runtime(profile)
 
-if(COMPILER_RT_BUILD_XRAY)
-  compiler_rt_build_runtime(xray)
-endif()
+# the following set is conditional to COMPILER_RT_BUILD_XRAY
+# (via COMPILER_RT_HAS_* in config-ix.cmake)
+compiler_rt_build_runtime(xray)
Index: include/CMakeLists.txt
===
--- include/CMakeLists.txt
+++ include/CMakeLists.txt
@@ -1,19 +1,43 @@
-set(SANITIZER_HEADERS
-  sanitizer/allocator_interface.h
-  sanitizer/asan_interface.h
-  sanitizer/common_interface_defs.h
-  sanitizer/coverage_interface.h
-  sanitizer/dfsan_interface.h
-  sanitizer/esan_interface.h
-  sanitizer/linux_syscall_hooks.h
-  sanitizer/lsan_interface.h
-  sanitizer/msan_interface.h
-  sanitizer/tsan_interface.h
-  sanitizer/tsan_interface_atomic.h)
+set(SANITIZER_HEADERS)
+if(COMPILER_RT_HAS_SANITIZER_COMMON)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/allocator_interface.h
+   sanitizer/common_interface_defs.h
+   sanitizer/coverage_interface.h
+   sanitizer/linux_syscall_hooks.h)
+endif()
+if(COMPILER_RT_HAS_ASAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/asan_interface.h)
+endif()
+if(COMPILER_RT_HAS_DFSAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/dfsan_interface.h)
+endif()
+if(COMPILER_RT_HAS_ESAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/esan_interface.h)
+endif()
+if(COMPILER_RT_HAS_LSAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/lsan_interface.h)
+endif()
+if(COMPILER_RT_HAS_MSAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/msan_interface.h)
+endif()
+if(COMPILER_RT_HAS_TSAN)
+  list(APPEND SANITIZER_HEADERS
+   sanitizer/tsan_interface.h
+   sanitizer/tsan_interface_atomic.h)
+endif()
 
-set(XRAY_HEADERS
-  xray/xray_interface.h
-  xray/xray_log_interface.h)
+set(XRAY_HEADERS)
+if(COMPILER_RT_BUILD_XRAY)
+  list(APPEND XRAY_HEADERS
+   xray/xray_interface.h
+   xray/xray_log_interface.h)
+endif()
 
 set(COMPILER_RT_HEADERS
   ${SANITIZER_HEADERS}
Index: cmake/config-ix.cmake
=

Re: D30739: [OpenMP] "declare simd" for AArch64 Advanced SIMD.

[Francesco Petrogalli via cfe-commits]

On 23 Mar 2017, at 17:31, Tian, Xinmin  wrote:

> What is the error in the spec?
>

Hi Xinmin - there is nothing wrong with the specs for x86.

The problem is that the way the CDT is computed is not optimal for AArch64. We 
are in the process of re-defining it specifically for AArch64.

Francesco

> Thanks,
> Xinmin
>
> -Original Message-
> From: Francesco Petrogalli via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Thursday, March 23, 2017 10:29 AM
> To: francesco.petroga...@arm.com; cber...@us.ibm.com; acja...@us.ibm.com; 
> hahnf...@itc.rwth-aachen.de; a.bat...@hotmail.com
> Cc: Tian, Xinmin ; florian.h...@arm.com; 
> roger.ferreriba...@arm.com; amara.emer...@arm.com; renato.go...@linaro.org; 
> cfe-commits@lists.llvm.org
> Subject: [PATCH] D30739: [OpenMP] "declare simd" for AArch64 Advanced SIMD.
>
> fpetrogalli planned changes to this revision.
> fpetrogalli added a comment.
>
> Dear all,
>
> thank you for reviewing this patch. We found out an error in the spec and 
> would like to fix it before things are used.
>
> I will soon update this patch.
>
> Thanks,
>
> Francesco
>
>
> https://reviews.llvm.org/D30739
>
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r298976 - [OpenCL] Added parsing for OpenCL vector types.

[Anastasia Stulova via cfe-commits]

It seems like the following change causes the failures:



diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp

index 4dcdfbf..e1439d6 100644

--- a/lib/Parse/ParseExpr.cpp

+++ b/lib/Parse/ParseExpr.cpp

@@ -2379,10 +2427,13 @@ Parser::ParseParenExpression(ParenParseOption 
&ExprType, bool stopIfCastExpr,

 }

 // Parse the cast-expression that follows it next.

+// isVectorLiteral = true will make sure we don't parse any

+// Postfix expression yet

 // TODO: For cast expression with CastTy.

 Result = ParseCastExpression(/*isUnaryExpression=*/false,

  /*isAddressOfOperand=*/false,

- /*isTypeCast=*/IsTypeCast);

+ /*isTypeCast=*/IsTypeCast,

+ /*isVectorLiteral=*/true);



Although it appears in the review as well, but did you intend to change this?



Just FYI, passing "make check-clang" is a minimal testing requirement for Clang 
changes. Could you, please, try to verify it for your future commits. ;)



Thanks!

Anastasia

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Egor 
Churaev via cfe-commits
Sent: 29 March 2017 06:51
To: Dean Michael Berris
Cc: cfe-commits@lists.llvm.org
Subject: Re: r298976 - [OpenCL] Added parsing for OpenCL vector types.

I see it. I'm reverting this patch and I'll investigate why it has happend.

2017-03-29 8:43 GMT+03:00 Dean Michael Berris 
mailto:dber...@google.com>>:
This seems to have broken multiple builds.

On Wed, Mar 29, 2017 at 4:20 PM Egor Churaev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: echuraev
Date: Wed Mar 29 00:08:18 2017
New Revision: 298976

URL: http://llvm.org/viewvc/llvm-project?rev=298976&view=rev
Log:
[OpenCL] Added parsing for OpenCL vector types.

Reviewers: cfe-commits, Anastasia

Reviewed By: Anastasia

Subscribers: yaxunl, bader

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

Added:
cfe/trunk/test/Parser/vector-cast-define.cl
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseExpr.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=298976&r1=298975&r2=298976&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Mar 29 00:08:18 2017
@@ -1449,10 +1449,12 @@ private:
   ExprResult ParseCastExpression(bool isUnaryExpression,
  bool isAddressOfOperand,
  bool &NotCastExpr,
- TypeCastState isTypeCast);
+ TypeCastState isTypeCast,
+ bool isVectorLiteral = false);
   ExprResult ParseCastExpression(bool isUnaryExpression,
  bool isAddressOfOperand = false,
- TypeCastState isTypeCast = NotTypeCast);
+ TypeCastState isTypeCast = NotTypeCast,
+ bool isVectorLiteral = false);

   /// Returns true if the next token cannot start an expression.
   bool isNotExpressionStart();

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=298976&r1=298975&r2=298976&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Mar 29 00:08:18 2017
@@ -473,12 +473,14 @@ Parser::ParseRHSOfBinaryExpression(ExprR
 ///
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
-   TypeCastState isTypeCast) {
+   TypeCastState isTypeCast,
+   bool isVectorLiteral) {
   bool NotCastExpr;
   ExprResult Res = ParseCastExpression(isUnaryExpression,
isAddressOfOperand,
NotCastExpr,
-   isTypeCast);
+   isTypeCast,
+   isVectorLiteral);
   if (NotCastExpr)
 Diag(Tok, diag::err_expected_expression);
   return Res;
@@ -694,7 +696,8 @@ class CastExpressionIdValidator : public
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
bool &NotCastExpr,
-   TypeCastState isTypeCast) {
+   TypeCastState isTypeCast,
+  

[PATCH] D26830: [libcxx] Add string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D26830#711870, @EricWF wrote:

> @AntonBikineev when will you be able to make he requested changes? I would 
> like to land this ASAP.


Will do that today


https://reviews.llvm.org/D26830



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

r298992 - Reapplied r298976 [OpenCL] Added parsing for OpenCL vector types.

[Egor Churaev via cfe-commits]

Author: echuraev
Date: Wed Mar 29 07:09:39 2017
New Revision: 298992

URL: http://llvm.org/viewvc/llvm-project?rev=298992&view=rev
Log:
Reapplied r298976 [OpenCL] Added parsing for OpenCL vector types.

Added:
cfe/trunk/test/Parser/vector-cast-define.cl
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseExpr.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=298992&r1=298991&r2=298992&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Mar 29 07:09:39 2017
@@ -1449,10 +1449,12 @@ private:
   ExprResult ParseCastExpression(bool isUnaryExpression,
  bool isAddressOfOperand,
  bool &NotCastExpr,
- TypeCastState isTypeCast);
+ TypeCastState isTypeCast,
+ bool isVectorLiteral = false);
   ExprResult ParseCastExpression(bool isUnaryExpression,
  bool isAddressOfOperand = false,
- TypeCastState isTypeCast = NotTypeCast);
+ TypeCastState isTypeCast = NotTypeCast,
+ bool isVectorLiteral = false);
 
   /// Returns true if the next token cannot start an expression.
   bool isNotExpressionStart();

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=298992&r1=298991&r2=298992&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Mar 29 07:09:39 2017
@@ -473,12 +473,14 @@ Parser::ParseRHSOfBinaryExpression(ExprR
 ///
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
-   TypeCastState isTypeCast) {
+   TypeCastState isTypeCast,
+   bool isVectorLiteral) {
   bool NotCastExpr;
   ExprResult Res = ParseCastExpression(isUnaryExpression,
isAddressOfOperand,
NotCastExpr,
-   isTypeCast);
+   isTypeCast,
+   isVectorLiteral);
   if (NotCastExpr)
 Diag(Tok, diag::err_expected_expression);
   return Res;
@@ -694,7 +696,8 @@ class CastExpressionIdValidator : public
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
bool &NotCastExpr,
-   TypeCastState isTypeCast) {
+   TypeCastState isTypeCast,
+   bool isVectorLiteral) {
   ExprResult Res;
   tok::TokenKind SavedKind = Tok.getKind();
   NotCastExpr = false;
@@ -722,6 +725,9 @@ ExprResult Parser::ParseCastExpression(b
 Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/,
isTypeCast == IsTypeCast, CastTy, RParenLoc);
 
+if (isVectorLiteral)
+return Res;
+
 switch (ParenExprType) {
 case SimpleExpr:   break;// Nothing else to do.
 case CompoundStmt: break;  // Nothing else to do.
@@ -2350,6 +2356,48 @@ Parser::ParseParenExpression(ParenParseO
 return ParseCompoundLiteralExpression(Ty.get(), OpenLoc, RParenLoc);
   }
 
+  if (Tok.is(tok::l_paren)) {
+// This could be OpenCL vector Literals
+if (getLangOpts().OpenCL)
+{
+  TypeResult Ty;
+  {
+InMessageExpressionRAIIObject InMessage(*this, false);
+Ty = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
+  }
+  if(Ty.isInvalid())
+  {
+ return ExprError();
+  }
+  QualType QT = Ty.get().get().getCanonicalType();
+  if (QT->isVectorType())
+  {
+// We parsed '(' vector-type-name ')' followed by '('
+
+// Parse the cast-expression that follows it next.
+// isVectorLiteral = true will make sure we don't parse any
+// Postfix expression yet
+Result = ParseCastExpression(/*isUnaryExpression=*/false,
+ /*isAddressOfOperand=*/false,
+ /*isTypeCast=*/IsTypeCast,
+ /*isVectorLiteral=*/true);
+
+if (!Result.isInvalid()) {
+  Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
+ 

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: lib/Headers/avxintrin.h:1752
+///0x1e : Greater-than (ordered, non-signaling)
+///0x1f : True (unordered, signaling)
 /// \returns A 128-bit vector of [4 x float] containing the comparison results.

Minor: clean up the nonsignalling/non-sign versions to non-signaling etc.


https://reviews.llvm.org/D31428



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

[PATCH] D31321: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args

[Egor Churaev via Phabricator via cfe-commits]

echuraev added inline comments.



Comment at: test/CodeGenOpenCL/kernel-arg-info.cl:66
 // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"}
 // ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}

Anastasia wrote:
> Could we modify the test to check that const and volatile are added for the 
> pointers though?
Added test case only for volatile pointer because const is checked in the Z 
argument of foo function.


https://reviews.llvm.org/D31321



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

[PATCH] D31321: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args

[Egor Churaev via Phabricator via cfe-commits]

echuraev updated this revision to Diff 93357.
echuraev marked an inline comment as done.

https://reviews.llvm.org/D31321

Files:
  lib/CodeGen/CodeGenFunction.cpp
  test/CodeGenOpenCL/kernel-arg-info.cl


Index: test/CodeGenOpenCL/kernel-arg-info.cl
===
--- test/CodeGenOpenCL/kernel-arg-info.cl
+++ test/CodeGenOpenCL/kernel-arg-info.cl
@@ -2,7 +2,8 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -triple 
spir-unknown-unknown -cl-kernel-arg-info | FileCheck %s -check-prefix ARGINFO
 
 kernel void foo(__global int * restrict X, const int Y, 
-volatile int anotherArg, __constant float * restrict Z) {
+volatile int anotherArg, __constant float * restrict Z,
+__global volatile int * V) {
   *X = Y + anotherArg;
 }
 // CHECK: define spir_kernel void @foo{{[^!]+}}
@@ -60,11 +61,11 @@
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
 
-// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2}
-// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none"}
-// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"}
-// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}
+// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2, i32 1}
+// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none"}
+// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*", !"int*"}
+// CHECK: ![[MD14]] = !{!"restrict", !"", !"", !"restrict const", !"volatile"}
+// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z", !"V"}
 // CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1}
 // CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", 
!"read_write"}
 // CHECK: ![[MD23]] = !{!"image1d_t", !"image2d_t", !"image2d_array_t", 
!"image1d_t"}
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -610,11 +610,6 @@
 
   argBaseTypeNames.push_back(llvm::MDString::get(Context, baseTypeName));
 
-  // Get argument type qualifiers:
-  if (ty.isConstQualified())
-typeQuals = "const";
-  if (ty.isVolatileQualified())
-typeQuals += typeQuals.empty() ? "volatile" : " volatile";
   if (isPipe)
 typeQuals = "pipe";
 }


Index: test/CodeGenOpenCL/kernel-arg-info.cl
===
--- test/CodeGenOpenCL/kernel-arg-info.cl
+++ test/CodeGenOpenCL/kernel-arg-info.cl
@@ -2,7 +2,8 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -triple spir-unknown-unknown -cl-kernel-arg-info | FileCheck %s -check-prefix ARGINFO
 
 kernel void foo(__global int * restrict X, const int Y, 
-volatile int anotherArg, __constant float * restrict Z) {
+volatile int anotherArg, __constant float * restrict Z,
+__global volatile int * V) {
   *X = Y + anotherArg;
 }
 // CHECK: define spir_kernel void @foo{{[^!]+}}
@@ -60,11 +61,11 @@
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
 
-// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2}
-// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none"}
-// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"}
-// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}
+// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2, i32 1}
+// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none"}
+// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*", !"int*"}
+// CHECK: ![[MD14]] = !{!"restrict", !"", !"", !"restrict const", !"volatile"}
+// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z", !"V"}
 // CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1}
 // CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", !"read_write"}
 // CHECK: ![[MD23]] = !{!"image1d_t", !"image2d_t", !"image2d_array_t", !"image1d_t"}
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -610,11 +610,6 @@
 
   argBaseTypeNames.push_back(llvm::MDString::get(Context, baseTypeName));
 
-  // Get argument type qualifiers:
-  if (ty.isConstQualified())
-typeQuals = "const";
-  if (ty.isVolatileQualified())
-typeQuals += typeQuals.empty() ? "volatile" : " volatile";
   if (isPipe)
 typeQuals = "pipe";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

[Sanjay Patel via Phabricator via cfe-commits]

spatel updated this revision to Diff 93366.
spatel added a comment.

Patch updated:
Standardize the spelling of "non-signaling" and don't abbreviate "unordered".


https://reviews.llvm.org/D31428

Files:
  lib/Headers/avxintrin.h

Index: lib/Headers/avxintrin.h
===
--- lib/Headers/avxintrin.h
+++ lib/Headers/avxintrin.h
@@ -1613,9 +1613,9 @@
 #define _CMP_NEQ_UQ   0x04 /* Not-equal (unordered, non-signaling)  */
 #define _CMP_NLT_US   0x05 /* Not-less-than (unordered, signaling)  */
 #define _CMP_NLE_US   0x06 /* Not-less-than-or-equal (unordered, signaling)  */
-#define _CMP_ORD_Q0x07 /* Ordered (nonsignaling)   */
+#define _CMP_ORD_Q0x07 /* Ordered (non-signaling)   */
 #define _CMP_EQ_UQ0x08 /* Equal (unordered, non-signaling)  */
-#define _CMP_NGE_US   0x09 /* Not-greater-than-or-equal (unord, signaling)  */
+#define _CMP_NGE_US   0x09 /* Not-greater-than-or-equal (unordered, signaling)  */
 #define _CMP_NGT_US   0x0a /* Not-greater-than (unordered, signaling)  */
 #define _CMP_FALSE_OQ 0x0b /* False (ordered, non-signaling)  */
 #define _CMP_NEQ_OQ   0x0c /* Not-equal (ordered, non-signaling)  */
@@ -1628,10 +1628,10 @@
 #define _CMP_UNORD_S  0x13 /* Unordered (signaling)  */
 #define _CMP_NEQ_US   0x14 /* Not-equal (unordered, signaling)  */
 #define _CMP_NLT_UQ   0x15 /* Not-less-than (unordered, non-signaling)  */
-#define _CMP_NLE_UQ   0x16 /* Not-less-than-or-equal (unord, non-signaling)  */
+#define _CMP_NLE_UQ   0x16 /* Not-less-than-or-equal (unordered, non-signaling)  */
 #define _CMP_ORD_S0x17 /* Ordered (signaling)  */
 #define _CMP_EQ_US0x18 /* Equal (unordered, signaling)  */
-#define _CMP_NGE_UQ   0x19 /* Not-greater-than-or-equal (unord, non-sign)  */
+#define _CMP_NGE_UQ   0x19 /* Not-greater-than-or-equal (unordered, non-signaling)  */
 #define _CMP_NGT_UQ   0x1a /* Not-greater-than (unordered, non-signaling)  */
 #define _CMP_FALSE_OS 0x1b /* False (ordered, signaling)  */
 #define _CMP_NEQ_OS   0x1c /* Not-equal (ordered, signaling)  */
@@ -1660,17 +1660,38 @@
 /// \param c
 ///An immediate integer operand, with bits [4:0] specifying which comparison
 ///operation to use: \n
-///00h, 08h, 10h, 18h: Equal \n
-///01h, 09h, 11h, 19h: Less than \n
-///02h, 0Ah, 12h, 1Ah: Less than or equal / Greater than or equal
-///(swapped operands) \n
-///03h, 0Bh, 13h, 1Bh: Unordered \n
-///04h, 0Ch, 14h, 1Ch: Not equal \n
-///05h, 0Dh, 15h, 1Dh: Not less than / Not greater than
-///(swapped operands) \n
-///06h, 0Eh, 16h, 1Eh: Not less than or equal / Not greater than or equal
-///(swapped operands) \n
-///07h, 0Fh, 17h, 1Fh: Ordered
+///0x00 : Equal (ordered, non-signaling)
+///0x01 : Less-than (ordered, signaling)
+///0x02 : Less-than-or-equal (ordered, signaling)
+///0x03 : Unordered (non-signaling)
+///0x04 : Not-equal (unordered, non-signaling)
+///0x05 : Not-less-than (unordered, signaling)
+///0x06 : Not-less-than-or-equal (unordered, signaling)
+///0x07 : Ordered (non-signaling)
+///0x08 : Equal (unordered, non-signaling)
+///0x09 : Not-greater-than-or-equal (unordered, signaling)
+///0x0a : Not-greater-than (unordered, signaling)
+///0x0b : False (ordered, non-signaling)
+///0x0c : Not-equal (ordered, non-signaling)
+///0x0d : Greater-than-or-equal (ordered, signaling)
+///0x0e : Greater-than (ordered, signaling)
+///0x0f : True (unordered, non-signaling)
+///0x10 : Equal (ordered, signaling)
+///0x11 : Less-than (ordered, non-signaling)
+///0x12 : Less-than-or-equal (ordered, non-signaling)
+///0x13 : Unordered (signaling)
+///0x14 : Not-equal (unordered, signaling)
+///0x15 : Not-less-than (unordered, non-signaling)
+///0x16 : Not-less-than-or-equal (unordered, non-signaling)
+///0x17 : Ordered (signaling)
+///0x18 : Equal (unordered, signaling)
+///0x19 : Not-greater-than-or-equal (unordered, non-signaling)
+///0x1a : Not-greater-than (unordered, non-signaling)
+///0x1b : False (ordered, signaling)
+///0x1c : Not-equal (ordered, signaling)
+///0x1d : Greater-than-or-equal (ordered, non-signaling)
+///0x1e : Greater-than (ordered, non-signaling)
+///0x1f : True (unordered, signaling)
 /// \returns A 128-bit vector of [2 x double] containing the comparison results.
 #define _mm_cmp_pd(a, b, c) __extension__ ({ \
   (__m128d)__builtin_ia32_cmppd((__v2df)(__m128d)(a), \
@@ -1697,17 +1718,38 @@
 /// \param c
 ///An immediate integer operand, with bits [4:0] specifying which comparison
 ///operation to use: \n
-///00h, 08h, 10h, 18h: Equal \n
-///01h, 09h, 11h, 19h: Less than \n
-///02h, 0Ah, 12h, 1Ah: Less than or equal / Greater than or equal
-///(swapped operands) \n
-///03h, 0Bh, 13h, 1Bh: Unordered \n
-///04h, 0Ch, 14

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Egor Churaev via Phabricator via cfe-commits]

echuraev updated this revision to Diff 93368.
echuraev marked 2 inline comments as done.

https://reviews.llvm.org/D30643

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  test/Parser/opencl-atomics-cl20.cl
  test/SemaOpenCL/atomic-init.cl


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can only be assigned 
to a variable in global adress space}}
+  private atomic_int a3 = 0; // expected-error {{atomic variable can only be 
assigned to a variable in global adress space}}
+  local atomic_int a4 = 0; // expected-error {{'__local' variable cannot have 
an initializer}}
+  global atomic_int a5 = 0; // expected-error {{function scope variable cannot 
be declared in global address space}}
+  static global atomic_int a6 = 0;
+}
Index: test/Parser/opencl-atomics-cl20.cl
===
--- test/Parser/opencl-atomics-cl20.cl
+++ test/Parser/opencl-atomics-cl20.cl
@@ -67,7 +67,7 @@
   foo(&i);
 // OpenCL v2.0 s6.13.11.8, arithemtic operations are not permitted on atomic 
types.
   i++; // expected-error {{invalid argument type 'atomic_int' (aka 
'_Atomic(int)') to unary expression}}
-  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant in the declaration statement in the program scope}}
+  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant}}
   i += 1; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'int')}}
   i = i + i; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'atomic_int')}}
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6502,6 +6502,20 @@
   << Init->getSourceRange();
   }
 
+  // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global 
scope
+  QualType ETy = Entity.getType();
+  Qualifiers TyQualifiers = ETy.getQualifiers();
+  bool HasGlobalAS = TyQualifiers.hasAddressSpace() &&
+ TyQualifiers.getAddressSpace() == LangAS::opencl_global;
+
+  if (S.getLangOpts().OpenCLVersion >= 200 &&
+  ETy->isAtomicType() && !HasGlobalAS &&
+  Entity.getKind() == InitializedEntity::EK_Variable && Args.size() > 0) {
+S.Diag(Args[0]->getLocStart(), diag::err_opencl_atomic_init) << 1 <<
+SourceRange(Entity.getDecl()->getLocStart(), Args[0]->getLocEnd());
+return ExprError();
+  }
+
   // Diagnose cases where we initialize a pointer to an array temporary, and 
the
   // pointer obviously outlives the temporary.
   if (Args.size() == 1 && Args[0]->getType()->isArrayType() &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2,7 +2,7 @@
 if (LHSTy->isAtomicType() || RHSTy->isAtomicType()) {
   SourceRange SR(LHSExpr->getLocStart(), RHSExpr->getLocEnd());
   if (BO_Assign == Opc)
-Diag(OpLoc, diag::err_atomic_init_constant) << SR;
+Diag(OpLoc, diag::err_opencl_atomic_init) << 0 << SR;
   else
 ResultTy = InvalidOperands(OpLoc, LHS, RHS);
   return ExprError();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8284,9 +8284,9 @@
   "return value cannot be qualified with address space">;
 def err_opencl_constant_no_init : Error<
   "variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
-  "atomic variable can only be assigned to a compile time constant"
-  " in the declaration statement in the program scope">;
+def err_opencl_atomic_init: Error<
+  "atomic variable can only be assigned to a %select{compile time constant|"
+  "variable in global adress space}0">;
 def err_opencl_implicit_vector_conversion : Error<
   "implicit conversions between vector types (%0 and %1) are not permitted">;
 def err_opencl_invalid_type_array : Error<


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

- Use pushCleanup to emit freeing coroutine memory on normal and EH exits.
- Surround emitted code with CodeGenFunction::RunCleanupsScope.


https://reviews.llvm.org/D31460

Files:
  lib/CodeGen/CGCoroutine.cpp
  test/CodeGenCoroutines/coro-cleanup.cpp
  test/CodeGenCoroutines/coro-return.cpp

Index: test/CodeGenCoroutines/coro-return.cpp
===
--- test/CodeGenCoroutines/coro-return.cpp
+++ test/CodeGenCoroutines/coro-return.cpp
@@ -1,35 +1,27 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++1z -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
-namespace std {
-namespace experimental {
-template 
-struct coroutine_traits;
+namespace std::experimental {
+template  struct coroutine_traits;
 
-template 
-struct coroutine_handle {
+template  struct coroutine_handle {
   coroutine_handle() = default;
   static coroutine_handle from_address(void *) { return {}; }
 };
-
-template <>
-struct coroutine_handle {
+template <> struct coroutine_handle {
   static coroutine_handle from_address(void *) { return {}; }
   coroutine_handle() = default;
   template 
   coroutine_handle(coroutine_handle) {}
 };
-
-}
 }
 
 struct suspend_always {
   bool await_ready();
   void await_suspend(std::experimental::coroutine_handle<>);
   void await_resume();
 };
 
-template<>
-struct std::experimental::coroutine_traits {
+template <> struct std::experimental::coroutine_traits {
   struct promise_type {
 void get_return_object();
 suspend_always initial_suspend();
Index: test/CodeGenCoroutines/coro-cleanup.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-cleanup.cpp
@@ -0,0 +1,74 @@
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) { return {}; }
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) {}
+};
+}
+
+struct suspend_always {
+  bool await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+
+template <> struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+promise_type();
+~promise_type();
+void unhandled_exception();
+  };
+};
+
+struct Cleanup { ~Cleanup(); };
+void may_throw();
+
+// CHECK: define void @_Z1fv(
+void f() {
+  // CHECK: call i8* @_Znwm(i64
+
+  // If promise constructor throws, check that we free the memory.
+
+  // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJvEE12promise_typeC1Ev(
+  // CHECK-NEXT: to label %{{.+}} unwind label %[[DeallocPad:.+]]
+
+  Cleanup cleanup;
+  may_throw();
+
+  // if may_throw throws, check that we destroy the promise and free the memory.
+
+  // CHECK: invoke void @_Z9may_throwv(
+  // CHECK-NEXT: to label %{{.+}} unwind label %[[PromDtorPad:.+]]
+
+  // CHECK: [[DeallocPad]]:
+  // CHECK-NEXT: landingpad
+  // CHECK-NEXT:   cleanup
+  // CHECK: br label %[[Dealloc:.+]]
+
+  // CHECK: [[PromDtorPad]]:
+  // CHECK-NEXT: landingpad
+  // CHECK-NEXT:   cleanup
+  // CHECK: call void @_ZN7CleanupD1Ev(%struct.Cleanup*
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvEE12promise_typeD1Ev(
+  // CHECK: br label %[[Dealloc]]
+
+  // CHECK: [[Dealloc]]:
+  // CHECK:   %[[Mem:.+]] = call i8* @llvm.coro.free(
+  // CHECK:   call void @_ZdlPv(i8* %[[Mem]])
+
+  co_return;
+}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -215,6 +215,19 @@
   EmitBranchThroughCleanup(CurCoro.Data->FinalJD);
 }
 
+namespace {
+// Make sure to call coro.delete on scope exit.
+struct CallCoroDelete final : public EHScopeStack::Cleanup {
+  Stmt *Deallocate;
+
+  // TODO: Wrap deallocate in if(coro.free(...)) Deallocate.
+  void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+  }
+  explicit CallCoroDelete(Stmt *DeallocStmt) : Deallocate(DeallocStmt) {}
+};
+}
+
 void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
   auto *NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
   auto &TI = CGM.getContext().getTargetInfo();
@@ -248,26 +261,28 

[Driver] Add option to print the resource directory

[Moore, Catherine via cfe-commits]

Hi,

I tried to submit this patch via Phabricator yesterday, but I don't think it 
made it to the list:

https://reviews.llvm.org/D31447

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

Re: r298913 - Added `applyAtomicChanges` function.

[Eric Liu via cfe-commits]

Hi Juergen, thanks for taking care of this, but I'm wondering if this build
bot is using a different set of build rules? The error message says
"Clang_Tooling
-> Clang_Format -> Clang_Tooling"; however, the actual dependency is
clangToolingRefactor -> clangFormat -> clangToolingCore, which seems fine
for me. I guess the module build uses build rules with lower granularity.

- Eric

On Wed, Mar 29, 2017 at 2:39 AM Juergen Ributzka 
wrote:

> I reverted the commit in r298967. Please fix the cyclic dependency issue
> found here:
>
> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-modulesRDA_build/4776/
>
> Cheers,
> Juergen
>
> On Tue, Mar 28, 2017 at 6:05 AM, Eric Liu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: ioeric
> Date: Tue Mar 28 08:05:32 2017
> New Revision: 298913
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298913&view=rev
> Log:
> Added `applyAtomicChanges` function.
>
> Summary: ... which applies a set of `AtomicChange`s on code.
>
> Reviewers: klimek, djasper
>
> Reviewed By: djasper
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D30777
>
> Modified:
> cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
> cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
> cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>
> Modified: cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h?rev=298913&r1=298912&r2=298913&view=diff
>
> ==
> --- cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h (original)
> +++ cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h Tue Mar 28
> 08:05:32 2017
> @@ -16,6 +16,7 @@
>  #define LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
>
>  #include "clang/Basic/SourceManager.h"
> +#include "clang/Format/Format.h"
>  #include "clang/Tooling/Core/Replacement.h"
>  #include "llvm/ADT/StringRef.h"
>  #include "llvm/Support/Error.h"
> @@ -123,6 +124,39 @@ private:
>tooling::Replacements Replaces;
>  };
>
> +// Defines specs for applying changes.
> +struct ApplyChangesSpec {
> +  // If true, cleans up redundant/erroneous code around changed code with
> +  // clang-format's cleanup functionality, e.g. redundant commas around
> deleted
> +  // parameter or empty namespaces introduced by deletions.
> +  bool Cleanup = true;
> +
> +  format::FormatStyle Style = format::getNoStyle();
> +
> +  // Options for selectively formatting changes with clang-format:
> +  // kAll: Format all changed lines.
> +  // kNone: Don't format anything.
> +  // kViolations: Format lines exceeding the `ColumnLimit` in `Style`.
> +  enum FormatOption { kAll, kNone, kViolations };
> +
> +  FormatOption Format = kNone;
> +};
> +
> +/// \brief Applies all AtomicChanges in \p Changes to the \p Code.
> +///
> +/// This completely ignores the file path in each change and replaces
> them with
> +/// \p FilePath, i.e. callers are responsible for ensuring all changes
> are for
> +/// the same file.
> +///
> +/// \returns The changed code if all changes are applied successfully;
> +/// otherwise, an llvm::Error carrying llvm::StringError is returned (the
> Error
> +/// message can be converted to string with `llvm::toString()` and the
> +/// error_code should be ignored).
> +llvm::Expected
> +applyAtomicChanges(llvm::StringRef FilePath, llvm::StringRef Code,
> +   llvm::ArrayRef Changes,
> +   const ApplyChangesSpec &Spec);
> +
>  } // end namespace tooling
>  } // end namespace clang
>
>
> Modified: cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp?rev=298913&r1=298912&r2=298913&view=diff
>
> ==
> --- cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp (original)
> +++ cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp Tue Mar 28 08:05:32
> 2017
> @@ -84,6 +84,116 @@ template <> struct MappingTraits
>  namespace clang {
>  namespace tooling {
> +namespace {
> +
> +// Returns true if there is any line that violates \p ColumnLimit in range
> +// [Start, End].
> +bool violatesColumnLimit(llvm::StringRef Code, unsigned ColumnLimit,
> + unsigned Start, unsigned End) {
> +  auto StartPos = Code.rfind('\n', Start);
> +  StartPos = (StartPos == llvm::StringRef::npos) ? 0 : StartPos + 1;
> +
> +  auto EndPos = Code.find("\n", End);
> +  if (EndPos == llvm::StringRef::npos)
> +EndPos = Code.size();
> +
> +  llvm::SmallVector Lines;
> +  Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
> +  for (llvm::StringRef Line : Lines)
> +if (Line.size() > ColumnLimit)
> +  return true;
> +  return false;
> +}
> +
> +std::vector
> +getRangesForFormating(llvm::StringRef Code, unsigned ColumnLimit,
> +  

[PATCH] D31417: [OpenMP] Add support for omp simd pragmas without runtime

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

Hi Graham,

I don't know much about Clang's machinery, but would it be possible to have 
`-fopenmp-simd` generate the same handler, but with restrictions? I fear this 
slight duplication could get considerably worse as we support more and more 
"non-RT" OMP pragmas.

Alternatively, if this is for testing purposes, we have another pragma which 
does exactly the same thing as `omp simd`, which are the Clang vectorizer 
pragmas (http://llvm.org/docs/Vectorizers.html#pragma-loop-hint-directives).

cheers,
--renato


https://reviews.llvm.org/D31417



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

[PATCH] D31417: [OpenMP] Add support for omp simd pragmas without runtime

[Graham Hunter via Phabricator via cfe-commits]

huntergr added a comment.

Hi Renato,

In https://reviews.llvm.org/D31417#713162, @rengolin wrote:

> I don't know much about Clang's machinery, but would it be possible to have 
> `-fopenmp-simd` generate the same handler, but with restrictions? I fear this 
> slight duplication could get considerably worse as we support more and more 
> "non-RT" OMP pragmas.


Sure, I can combine the handlers and switch behaviour within that if that's 
preferable. The other alternative I thought of was to perform the filtering in 
ParseOpenMP.cpp instead, but I need to figure out how to delete or skip tokens 
there without cluttering up the rest of the OpenMP parsing.

I'll come up with a new version with the combined handler tomorrow.

> Alternatively, if this is for testing purposes, we have another pragma which 
> does exactly the same thing as `omp simd`, which are the Clang vectorizer 
> pragmas (http://llvm.org/docs/Vectorizers.html#pragma-loop-hint-directives).

This feature comes from user requests, and basically matches the functionality 
of other compilers (e.g. gcc).

Thanks for the comments.

-Graham


https://reviews.llvm.org/D31417



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

[PATCH] D31422: Add builder for libunwind docs

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r299003


https://reviews.llvm.org/D31422



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

[PATCH] D31417: [OpenMP] Add support for omp simd pragmas without runtime

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

In https://reviews.llvm.org/D31417#713171, @huntergr wrote:

> The other alternative I thought of was to perform the filtering in 
> ParseOpenMP.cpp instead, but I need to figure out how to delete or skip 
> tokens there without cluttering up the rest of the OpenMP parsing.


That was my first thought, yes, but I'm not sure how invasive this could get.

> This feature comes from user requests, and basically matches the 
> functionality of other compilers (e.g. gcc).

Right, in this case, I think we have a stronger motivation to make that an 
integral part of the existing OpenMP parser.

cheers,
--renato


https://reviews.llvm.org/D31417



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

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+  if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {

mgehre wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > I think this would make more sense lifted into an AST matcher -- there 
> > > > are bound to be a *lot* of binary operators, so letting the matcher 
> > > > memoize things is likely to give better performance.
> > > Any reasons not to do this on the lexer level?
> > Technical reasons? None.
> > User-experience reasons? We wouldn't want this to be on by default (I don't 
> > think) and we usually don't implement off-by-default diagnostics in Clang. 
> > I think a case could be made for doing it in the Lexer if the performance 
> > is really that bad with a clang-tidy check and we couldn't improve it here, 
> > though.
> Do I correctly understand that "doing this on lexer level" would mean to 
> implement this as a warning directly into clang? If yes, would it be possible 
> to generate fixits and have them possibly applied automatically (as it is the 
> case for clang-tidy)?
You are correct, that means implementing it as a warning in Clang. I believe 
you can still generate those fixits from lexer-level diagnostics, but have not 
verified it.

However, I don't think this diagnostic would be appropriate for Clang because 
it would have to be off by default.



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:68
+  if (PrimarySpelling != Spelling) {
+diag(OpLoc, "operator uses alternative spelling")
+<< FixItHint::CreateReplacement(TokenRange, PrimarySpelling);

mgehre wrote:
> aaron.ballman wrote:
> > This diagnostic doesn't help the user to understand what's wrong with their 
> > code (especially in the presence of multiple operators). Perhaps "'%0' is 
> > an alternative token spelling; consider using '%1'"
> > 
> > It would be nice if we could say "consider using %1 for ", but I'm 
> > really not certain why we would diagnose this code in the first place (it's 
> > purely a matter of stylistic choice, as I understand it).
> The main rational for this check is to enforce consistency and thus make it 
> easier to read and comprehend the code.
> I agree with your proposed diagnostics.
I think that enforcing consistency is a good rationale for having the check, 
but would second the suggestion that this check have an option to enforce the 
consistency one way or the other.

Then the diagnostic can be:

"'%0' is %select{an alternative token|a primary token}2; consider using '%1' 
for consistency"



Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:22
+void OperatorsRepresentationCheck::registerMatchers(MatchFinder *Finder) {
+  // We ignore implicit != operators in C++11 range-based for loops.
+  Finder->addMatcher(binaryOperator(unless(hasLHS(ignoringImpCasts(

I think that this check should only be performed in C++. In C, the alternative 
token spellings are implemented as macros and would require additional work to 
support.


https://reviews.llvm.org/D31308



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

[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.

[Doug Gregor via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D31378



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

[PATCH] D25157: [compiler-rt] [cmake] Respect COMPILER_RT_BUILD_* for libs, headers and tests

[Weiming Zhao via Phabricator via cfe-commits]

weimingz added a comment.

Looks good to me but I'm not very familiar with the build of sanitizer and xray.


https://reviews.llvm.org/D25157



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

[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno accepted this revision.
bruno added a comment.

Thanks for working on this! LGTM too


https://reviews.llvm.org/D31378



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

[PATCH] D31241: [Modules][PCH] Serialize #pragma pack

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Thanks Alex. LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D31241



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

Re: r298956 - Default enable the rtm feature only on skylake and later for now because Intel disabled the feature on some haswell and broadwell processors:

[Eric Christopher via cfe-commits]

OK, I went ahead and  did this in the backend as well in:

echristo@athyra ~/s/llvm> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
M lib/Target/X86/X86.td
Committed r298986

-eric

On Tue, Mar 28, 2017 at 5:49 PM Jim Grosbach  wrote:

> SGTM
>
> Sent from my iPhone
>
> On Mar 28, 2017, at 5:25 PM, Eric Christopher  wrote:
>
>
>
> On Tue, Mar 28, 2017 at 4:31 PM Craig Topper 
> wrote:
>
> So if you use -march=hsw the backend will think rtm is enabled, but clang
> will block the intrinsics in the frontend?
>
>
> Yeah. I've come to the conclusion that I think we should remove the
> feature from the backend too. Using the option/feature in the frontend (or
> during codegen of any sort) will turn on the instruction in the backend and
> we should just rely on the option/feature.
>
>
>
> Not sure what you mean by split the haswell and broadwell cpu.
>
>
> Into something similar to how the skylake cpu is configured in the front
> end with different feature sets. I don't really think it's a good idea
> here, but I thought I'd raise it.
>
> Thoughts?
>
> -eric
>
>
>
> ~Craig
>
> On Tue, Mar 28, 2017 at 4:18 PM, Eric Christopher 
> wrote:
>
> Hi Craig, Quentin, Jim,
>
> Just bringing this patch to your attention here. I haven't turned it off
> in the backend since some processors do support it and we want to allow the
> code generation, with this change we simply make the user use -mrtm to get
> the functionality for now.
>
> Question: Do we want to split the haswell and broadwell cpu to handle this
> different feature set or just leave it as an optional "enable it yourself"
> since it's an errata fix?
>
> I went ahead and committed with this question outstanding so that users
> won't get illegal instructions if they happen to have the affected units.
>
> -eric
>
>
> On Tue, Mar 28, 2017 at 4:15 PM Eric Christopher via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: echristo
> Date: Tue Mar 28 18:03:19 2017
> New Revision: 298956
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298956&view=rev
> Log:
> Default enable the rtm feature only on skylake and later for now because
> Intel disabled the feature on some haswell and broadwell processors:
>
>
> http://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/core-m-processor-family-spec-update.pdf
>
> the -mrtm option will still work normally.
>
> Modified:
> cfe/trunk/lib/Basic/Targets.cpp
> cfe/trunk/test/Preprocessor/predefined-arch-macros.c
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=298956&r1=298955&r2=298956&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Tue Mar 28 18:03:19 2017
> @@ -3194,6 +3194,7 @@ bool X86TargetInfo::initFeatureMap(
>  setFeatureEnabledImpl(Features, "mpx", true);
>  setFeatureEnabledImpl(Features, "sgx", true);
>  setFeatureEnabledImpl(Features, "clflushopt", true);
> +setFeatureEnabledImpl(Features, "rtm", true);
>  LLVM_FALLTHROUGH;
>case CK_Broadwell:
>  setFeatureEnabledImpl(Features, "rdseed", true);
> @@ -3204,7 +3205,6 @@ bool X86TargetInfo::initFeatureMap(
>  setFeatureEnabledImpl(Features, "lzcnt", true);
>  setFeatureEnabledImpl(Features, "bmi", true);
>  setFeatureEnabledImpl(Features, "bmi2", true);
> -setFeatureEnabledImpl(Features, "rtm", true);
>  setFeatureEnabledImpl(Features, "fma", true);
>  setFeatureEnabledImpl(Features, "movbe", true);
>  LLVM_FALLTHROUGH;
>
> Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=298956&r1=298955&r2=298956&view=diff
>
> ==
> --- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
> +++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Tue Mar 28
> 18:03:19 2017
> @@ -525,7 +525,6 @@
>  // CHECK_CORE_AVX2_M32: #define __PCLMUL__ 1
>  // CHECK_CORE_AVX2_M32: #define __POPCNT__ 1
>  // CHECK_CORE_AVX2_M32: #define __RDRND__ 1
> -// CHECK_CORE_AVX2_M32: #define __RTM__ 1
>  // CHECK_CORE_AVX2_M32: #define __SSE2__ 1
>  // CHECK_CORE_AVX2_M32: #define __SSE3__ 1
>  // CHECK_CORE_AVX2_M32: #define __SSE4_1__ 1
> @@ -555,7 +554,6 @@
>  // CHECK_CORE_AVX2_M64: #define __PCLMUL__ 1
>  // CHECK_CORE_AVX2_M64: #define __POPCNT__ 1
>  // CHECK_CORE_AVX2_M64: #define __RDRND__ 1
> -// CHECK_CORE_AVX2_M64: #define __RTM__ 1
>  // CHECK_CORE_AVX2_M64: #define __SSE2_MATH__ 1
>  // CHECK_CORE_AVX2_M64: #define __SSE2__ 1
>  // CHECK_CORE_AVX2_M64: #define __SSE3__ 1
> @@ -591,7 +589,6 @@
>  // CHECK_BROADWELL_M32: #define __POPCNT__ 1
>  // CHECK_BROADWELL_M32: #define __RDRND__ 1
>  // CHECK_BROADWELL_M32: #define __RDSEED__ 1
> -// CHECK_BROADWELL_M32: #de

r299007 - Test Commit

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 12:18:05 2017
New Revision: 299007

URL: http://llvm.org/viewvc/llvm-project?rev=299007&view=rev
Log:
Test Commit

Remove trailing whitespace.

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=299007&r1=299006&r2=299007&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Mar 29 12:18:05 2017
@@ -12243,7 +12243,7 @@ void Sema::DefineImplicitLambdaToBlockPo
   // Set the body of the conversion function.
   Stmt *ReturnS = Return.get();
   Conv->setBody(new (Context) CompoundStmt(Context, ReturnS,
-   Conv->getLocation(), 
+   Conv->getLocation(),
Conv->getLocation()));
   
   // We're done; notify the mutation listener, if any.


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

[PATCH] D29642: [OpenMP] Make OpenMP generated code for the NVIDIA device relocatable by default

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: test/Driver/openmp-offload.c:598
+// CHK-PTXAS: ptxas{{.*}}" "-c"
+// CHK-PTXAS-NEXT: /bin/cp

This path might not be correct on all systems. Do we really need this check?


Repository:
  rL LLVM

https://reviews.llvm.org/D29642



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

r299008 - [Objective-C] C++ Classes with __weak Members non-POD Types when using -fobjc-weak

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 12:31:42 2017
New Revision: 299008

URL: http://llvm.org/viewvc/llvm-project?rev=299008&view=rev
Log:
[Objective-C] C++ Classes with __weak Members non-POD Types when using 
-fobjc-weak

Summary: When adding an Objective-C retainable type member to a C++ class, also 
check the LangOpts.ObjCWeak flag and the lifetime qualifier so __weak qualified 
Objective-C pointer members cause the class to be a non-POD type with 
non-trivial special members, so the compiler always emits the necessary runtime 
calls for copying, moving, and destroying the weak member. Otherwise, 
Objective-C++ classes with weak Objective-C pointer members compiled with 
-fobjc-weak exhibit undefined behavior if the C++ class is classified as a POD 
type.

Reviewers: rsmith, benlangmuir, doug.gregor, rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGenObjCXX/objc-weak.mm
Modified:
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=299008&r1=299007&r2=299008&view=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Wed Mar 29 12:31:42 2017
@@ -722,9 +722,7 @@ void CXXRecordDecl::addedMember(Decl *D)
 ASTContext &Context = getASTContext();
 QualType T = Context.getBaseElementType(Field->getType());
 if (T->isObjCRetainableType() || T.isObjCGCStrong()) {
-  if (!Context.getLangOpts().ObjCAutoRefCount) {
-setHasObjectMember(true);
-  } else if (T.getObjCLifetime() != Qualifiers::OCL_ExplicitNone) {
+  if (T.hasNonTrivialObjCLifetime()) {
 // Objective-C Automatic Reference Counting:
 //   If a class has a non-static data member of Objective-C pointer
 //   type (or array thereof), it is a non-POD type and its
@@ -736,6 +734,8 @@ void CXXRecordDecl::addedMember(Decl *D)
 Data.PlainOldData = false;
 Data.HasTrivialSpecialMembers = 0;
 Data.HasIrrelevantDestructor = false;
+  } else if (!Context.getLangOpts().ObjCAutoRefCount) {
+setHasObjectMember(true);
   }
 } else if (!T.isCXX98PODType(Context))
   data().PlainOldData = false;

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=299008&r1=299007&r2=299008&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Mar 29 12:31:42 2017
@@ -4399,11 +4399,8 @@ BuildImplicitMemberInitializer(Sema &Sem
 }
   }
   
-  if (SemaRef.getLangOpts().ObjCAutoRefCount &&
-  FieldBaseElementType->isObjCRetainableType() &&
-  FieldBaseElementType.getObjCLifetime() != Qualifiers::OCL_None &&
-  FieldBaseElementType.getObjCLifetime() != Qualifiers::OCL_ExplicitNone) {
-// ARC:
+  if (FieldBaseElementType.hasNonTrivialObjCLifetime()) {
+// ARC and Weak:
 //   Default-initialize Objective-C pointers to NULL.
 CXXMemberInit
   = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Field, 

Added: cfe/trunk/test/CodeGenObjCXX/objc-weak.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/objc-weak.mm?rev=299008&view=auto
==
--- cfe/trunk/test/CodeGenObjCXX/objc-weak.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/objc-weak.mm Wed Mar 29 12:31:42 2017
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks 
-fobjc-weak -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+struct A { __weak id x; };
+
+id test0() {
+  A a;
+  A b = a;
+  A c(static_cast(b));
+  a = c;
+  c = static_cast(a);
+  return c.x;
+}
+
+// Copy Assignment Operator
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %struct.A* 
@_ZN1AaSERKS_(
+// CHECK:   [[THISADDR:%this.*]] = alloca [[A:.*]]*
+// CHECK:   [[OBJECTADDR:%.*]] = alloca [[A:.*]]*
+// CHECK:   [[THIS:%this.*]] = load [[A]]*, [[A]]** [[THISADDR]]
+// CHECK:   [[OBJECT:%.*]] = load [[A]]*, [[A]]** [[OBJECTADDR]]
+// CHECK:   [[T0:%.*]] = getelementptr inbounds [[A]], [[A]]* [[OBJECT]], 
i32 0, i32 0
+// CHECK-NEXT:  [[T1:%.*]] = call i8* @objc_loadWeak(i8** [[T0]])
+// CHECK-NEXT:  [[T2:%.*]] = getelementptr inbounds [[A]], [[A]]* [[THIS]], 
i32 0, i32 0
+// CHECK-NEXT:  [[T3:%.*]] = call i8* @objc_storeWeak(i8** [[T2]], i8* [[T1]])
+
+// Move Assignment Operator
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %struct.A* 
@_ZN1AaSEOS_(
+// CHECK:   [[THISADDR:%this.*]] = alloca [[A:.*]]*
+// CHECK:   [[OBJECTADDR:%.*]] = alloca [[A:.*]]*
+// CHECK:   [[THIS:%this.*]] = load [[A]]*, [[A

r299009 - [PCH] Attach instance's dependency collectors to PCH external AST sources.

[Graydon Hoare via cfe-commits]

Author: graydon
Date: Wed Mar 29 12:33:09 2017
New Revision: 299009

URL: http://llvm.org/viewvc/llvm-project?rev=299009&view=rev
Log:
[PCH] Attach instance's dependency collectors to PCH external AST sources.

Summary:
When a PCH is included via -include-pch, clang should treat the
current TU as dependent on the sourcefile that the PCH was generated from.

This is currently _partly_ accomplished by InitializePreprocessor calling
AddImplicitIncludePCH to synthesize an implicit #include of the sourcefile,
into the preprocessor's Predefines buffer.

For FrontendActions such as PreprocessOnlyAction (which is, curiously, what the
driver winds up running one of in response to a plain clang -M) this is
sufficient: the preprocessor cranks over its Predefines and emits a dependency
reference to the initial sourcefile.

For other FrontendActions (for example -emit-obj or -fsyntax-only) the
Predefines buffer is reset to the suggested predefines buffer from the PCH, so
the dependency edge is lost. The result is that clang emits a .d file in those
cases that lacks a reference to the .h file responsible for the input (and in
Swift's case, our .swiftdeps file winds up not including a reference to the
source file for a PCH bridging header.)

This patch fixes the problem by taking a different tack: ignoring the
Predefines buffer (which seems a bit like a hack anyways) and directly
attaching the CompilerInstance's DependencyCollectors (and legacy
DependencyFileGenerator) to the ASTReader for the external AST.

This approach is similar to the one chosen in earlier consultation with Bruno
and Ben, and I think it's the least-bad solution, given several options.

Reviewers: bruno, benlangmuir, doug.gregor

Reviewed By: bruno, doug.gregor

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/PCH/emit-dependencies.c
Modified:
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=299009&r1=299008&r2=299009&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Wed Mar 29 12:33:09 2017
@@ -662,6 +662,8 @@ public:
   bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
   const PCHContainerReader &PCHContainerRdr,
   ArrayRef> Extensions,
+  DependencyFileGenerator *DependencyFile,
+  ArrayRef> DependencyCollectors,
   void *DeserializationListener, bool OwnDeserializationListener,
   bool Preamble, bool UseGlobalModuleIndex);
 

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=299009&r1=299008&r2=299009&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Mar 29 12:33:09 2017
@@ -497,6 +497,8 @@ void CompilerInstance::createPCHExternal
   AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(),
   getPCHContainerReader(),
   getFrontendOpts().ModuleFileExtensions,
+  TheDependencyFileGenerator.get(),
+  DependencyCollectors,
   DeserializationListener,
   OwnDeserializationListener, Preamble,
   getFrontendOpts().UseGlobalModuleIndex);
@@ -507,6 +509,8 @@ IntrusiveRefCntPtr CompilerIn
 bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
 const PCHContainerReader &PCHContainerRdr,
 ArrayRef> Extensions,
+DependencyFileGenerator *DependencyFile,
+ArrayRef> DependencyCollectors,
 void *DeserializationListener, bool OwnDeserializationListener,
 bool Preamble, bool UseGlobalModuleIndex) {
   HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
@@ -524,6 +528,12 @@ IntrusiveRefCntPtr CompilerIn
   Reader->setDeserializationListener(
   static_cast(DeserializationListener),
   /*TakeOwnership=*/OwnDeserializationListener);
+
+  if (DependencyFile)
+DependencyFile->AttachToASTReader(*Reader);
+  for (auto &Listener : DependencyCollectors)
+Listener->attachToASTReader(*Reader);
+
   switch (Reader->ReadAST(Path,
   Preamble ? serialization::MK_Preamble
: serialization::MK_PCH,

Added: cfe/trunk/test/PCH/emit-dependencies.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/emit-dependencies.c?rev=299009&view=auto
==
--- cfe/trunk/test/PCH/emit-dependencies.c (added)
+++ cfe/trunk/test/PCH/emit-dependencies.c Wed Mar 29 12:33:09 2017
@@ -0,0 +1,9 @@
+// RUN: rm -f %t.pch
+/

[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.

[Graydon Hoare via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299009: [PCH] Attach instance's dependency collectors to PCH 
external AST sources. (authored by graydon).

Changed prior to commit:
  https://reviews.llvm.org/D31378?vs=93089&id=93387#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31378

Files:
  cfe/trunk/include/clang/Frontend/CompilerInstance.h
  cfe/trunk/lib/Frontend/CompilerInstance.cpp
  cfe/trunk/test/PCH/emit-dependencies.c


Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp
@@ -497,6 +497,8 @@
   AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(),
   getPCHContainerReader(),
   getFrontendOpts().ModuleFileExtensions,
+  TheDependencyFileGenerator.get(),
+  DependencyCollectors,
   DeserializationListener,
   OwnDeserializationListener, Preamble,
   getFrontendOpts().UseGlobalModuleIndex);
@@ -507,6 +509,8 @@
 bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
 const PCHContainerReader &PCHContainerRdr,
 ArrayRef> Extensions,
+DependencyFileGenerator *DependencyFile,
+ArrayRef> DependencyCollectors,
 void *DeserializationListener, bool OwnDeserializationListener,
 bool Preamble, bool UseGlobalModuleIndex) {
   HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
@@ -524,6 +528,12 @@
   Reader->setDeserializationListener(
   static_cast(DeserializationListener),
   /*TakeOwnership=*/OwnDeserializationListener);
+
+  if (DependencyFile)
+DependencyFile->AttachToASTReader(*Reader);
+  for (auto &Listener : DependencyCollectors)
+Listener->attachToASTReader(*Reader);
+
   switch (Reader->ReadAST(Path,
   Preamble ? serialization::MK_Preamble
: serialization::MK_PCH,
Index: cfe/trunk/include/clang/Frontend/CompilerInstance.h
===
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h
@@ -662,6 +662,8 @@
   bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
   const PCHContainerReader &PCHContainerRdr,
   ArrayRef> Extensions,
+  DependencyFileGenerator *DependencyFile,
+  ArrayRef> DependencyCollectors,
   void *DeserializationListener, bool OwnDeserializationListener,
   bool Preamble, bool UseGlobalModuleIndex);
 
Index: cfe/trunk/test/PCH/emit-dependencies.c
===
--- cfe/trunk/test/PCH/emit-dependencies.c
+++ cfe/trunk/test/PCH/emit-dependencies.c
@@ -0,0 +1,9 @@
+// RUN: rm -f %t.pch
+// RUN: %clang_cc1 -emit-pch -o %t.pch %S/Inputs/chain-decls1.h
+// RUN: %clang_cc1 -include-pch %t.pch -fsyntax-only -MT %s.o -dependency-file 
- %s | FileCheck %s
+// CHECK: Inputs/chain-decls1.h
+
+int main() {
+  f();
+  return 0;
+}


Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp
@@ -497,6 +497,8 @@
   AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(),
   getPCHContainerReader(),
   getFrontendOpts().ModuleFileExtensions,
+  TheDependencyFileGenerator.get(),
+  DependencyCollectors,
   DeserializationListener,
   OwnDeserializationListener, Preamble,
   getFrontendOpts().UseGlobalModuleIndex);
@@ -507,6 +509,8 @@
 bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
 const PCHContainerReader &PCHContainerRdr,
 ArrayRef> Extensions,
+DependencyFileGenerator *DependencyFile,
+ArrayRef> DependencyCollectors,
 void *DeserializationListener, bool OwnDeserializationListener,
 bool Preamble, bool UseGlobalModuleIndex) {
   HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
@@ -524,6 +528,12 @@
   Reader->setDeserializationListener(
   static_cast(DeserializationListener),
   /*TakeOwnership=*/OwnDeserializationListener);
+
+  if (DependencyFile)
+DependencyFile->AttachToASTReader(*Reader);
+  for (auto &Listener : DependencyCollectors)
+Listener->attachToASTReader(*Reader);
+
   switch (Reader->ReadAST(Path,
   Preamble ? serialization::MK_Preamble
: serialization::MK_PCH,
Index: cfe/trunk/include/clang/Frontend/CompilerInstance.h
===
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h
@@ -662,6 +662,8 @@
   bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
   const PCHContainerReade

[PATCH] D31440: PR32382: Adapt to LLVM changes in DIExpression.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I'm a bit confused - the alloca was only emitted at -O0, by the looks of it. 
Presumably it's pessimizing in some way at higher optimization levels? Or is 
that not the case?

Also, it looks like this change lost the "if > gmlt" test, so might cause 
variable declarations (& thus types) to leak into GMLT, which is undesirable.


https://reviews.llvm.org/D31440



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

r299010 - [Objective-C] Fix __weak type traits with -fobjc-weak

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 12:40:35 2017
New Revision: 299010

URL: http://llvm.org/viewvc/llvm-project?rev=299010&view=rev
Log:
[Objective-C] Fix __weak type traits with -fobjc-weak

Summary: Similar to ARC, in ObjCWeak Objective-C object pointers qualified with 
a weak lifetime are not POD or trivial types. Update the type trait code to 
reflect this. Copy and adapt the arc-type-traits.mm test case to verify 
correctness.

Reviewers: rsmith, doug.gregor, rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/SemaObjCXX/objc-weak-type-traits.mm
Modified:
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=299010&r1=299009&r2=299010&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Wed Mar 29 12:40:35 2017
@@ -2023,20 +2023,8 @@ bool QualType::isCXX98PODType(const ASTC
   if ((*this)->isIncompleteType())
 return false;
 
-  if (Context.getLangOpts().ObjCAutoRefCount) {
-switch (getObjCLifetime()) {
-case Qualifiers::OCL_ExplicitNone:
-  return true;
-  
-case Qualifiers::OCL_Strong:
-case Qualifiers::OCL_Weak:
-case Qualifiers::OCL_Autoreleasing:
-  return false;
-
-case Qualifiers::OCL_None:
-  break;
-}
-  }
+  if (hasNonTrivialObjCLifetime())
+return false;
   
   QualType CanonicalType = getTypePtr()->CanonicalType;
   switch (CanonicalType->getTypeClass()) {
@@ -2085,22 +2073,8 @@ bool QualType::isTrivialType(const ASTCo
   if ((*this)->isIncompleteType())
 return false;
   
-  if (Context.getLangOpts().ObjCAutoRefCount) {
-switch (getObjCLifetime()) {
-case Qualifiers::OCL_ExplicitNone:
-  return true;
-  
-case Qualifiers::OCL_Strong:
-case Qualifiers::OCL_Weak:
-case Qualifiers::OCL_Autoreleasing:
-  return false;
-  
-case Qualifiers::OCL_None:
-  if ((*this)->isObjCLifetimeType())
-return false;
-  break;
-}
-  }
+  if (hasNonTrivialObjCLifetime())
+return false;
   
   QualType CanonicalType = getTypePtr()->CanonicalType;
   if (CanonicalType->isDependentType())
@@ -2137,22 +2111,8 @@ bool QualType::isTriviallyCopyableType(c
   if ((*this)->isArrayType())
 return Context.getBaseElementType(*this).isTriviallyCopyableType(Context);
 
-  if (Context.getLangOpts().ObjCAutoRefCount) {
-switch (getObjCLifetime()) {
-case Qualifiers::OCL_ExplicitNone:
-  return true;
-  
-case Qualifiers::OCL_Strong:
-case Qualifiers::OCL_Weak:
-case Qualifiers::OCL_Autoreleasing:
-  return false;
-  
-case Qualifiers::OCL_None:
-  if ((*this)->isObjCLifetimeType())
-return false;
-  break;
-}
-  }
+  if (hasNonTrivialObjCLifetime())
+return false;
 
   // C++11 [basic.types]p9
   //   Scalar types, trivially copyable class types, arrays of such types, and
@@ -2298,20 +2258,8 @@ bool QualType::isCXX11PODType(const ASTC
   if (ty->isDependentType())
 return false;
 
-  if (Context.getLangOpts().ObjCAutoRefCount) {
-switch (getObjCLifetime()) {
-case Qualifiers::OCL_ExplicitNone:
-  return true;
-  
-case Qualifiers::OCL_Strong:
-case Qualifiers::OCL_Weak:
-case Qualifiers::OCL_Autoreleasing:
-  return false;
-
-case Qualifiers::OCL_None:
-  break;
-}
-  }
+  if (hasNonTrivialObjCLifetime())
+return false;
 
   // C++11 [basic.types]p9:
   //   Scalar types, POD classes, arrays of such types, and cv-qualified

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=299010&r1=299009&r2=299010&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Mar 29 12:40:35 2017
@@ -4518,25 +4518,6 @@ static bool EvaluateUnaryTypeTrait(Sema
   }
 }
 
-/// \brief Determine whether T has a non-trivial Objective-C lifetime in
-/// ARC mode.
-static bool hasNontrivialObjCLifetime(QualType T) {
-  switch (T.getObjCLifetime()) {
-  case Qualifiers::OCL_ExplicitNone:
-return false;
-
-  case Qualifiers::OCL_Strong:
-  case Qualifiers::OCL_Weak:
-  case Qualifiers::OCL_Autoreleasing:
-return true;
-
-  case Qualifiers::OCL_None:
-return T->isObjCLifetimeType();
-  }
-
-  llvm_unreachable("Unknown ObjC lifetime qualifier");
-}
-
 static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
@@ -4630,10 +4611,9 @@ static bool evaluateTypeTrait(Sema &S, T
   return S.canThrow(Result.get()) == CT_Cannot;
 
 if (Kind == clang::TT_IsTriviallyConstru

[PATCH] D29644: [OpenMP] Pass -v to PTXAS if it was passed to the driver.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: test/Driver/openmp-offload.c:607
+// CHK-VERBOSE: ptxas{{.*}}" "-v"
+// CHK-VERBOSE-NEXT: /bin/cp

This path might not be correct on all systems. Do we really need this check?


Repository:
  rL LLVM

https://reviews.llvm.org/D29644



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

[PATCH] D29659: [OpenMP] Add flag for disabling the default generation of relocatable OpenMP target code for NVIDIA GPUs.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Having something with dashes behind other used prefixes (`CHK-PTXAS`, 
`CHK-PTXAS-C`, `CHK-PTXAS-C-RELO`) might not be optimal and break when other 
suffixes like `-NOT` or `-SAME` are added to lit. Please see inline about my 
suggestions but feel free to use others...




Comment at: test/Driver/openmp-offload.c:608
 
 // CHK-PTXAS: ptxas{{.*}}" "-c"
 // CHK-PTXAS-NEXT: /bin/cp

`CHK-PTXAS-DEFAULT` (to be changed in D29642)



Comment at: test/Driver/openmp-offload.c:617
+
+// CHK-PTXAS-C-NOT: ptxas{{.*}}" "-c"
+

`CHK-PTXAS-NORELO`



Comment at: test/Driver/openmp-offload.c:626
+
+// CHK-PTXAS-C-RELO: ptxas{{.*}}" "-c"
+

`CHK-PTXAS-RELO`



Comment at: test/Driver/openmp-offload.c:634
 
 // CHK-VERBOSE: ptxas{{.*}}" "-v"
 // CHK-VERBOSE-NEXT: /bin/cp

`CHK-PTXAS-VERBOSE` (to be changed in D29644)


Repository:
  rL LLVM

https://reviews.llvm.org/D29659



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

r299011 - [Objective-C] Fix "repeated use of weak" warning with -fobjc-weak

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 12:55:11 2017
New Revision: 299011

URL: http://llvm.org/viewvc/llvm-project?rev=299011&view=rev
Log:
[Objective-C] Fix "repeated use of weak" warning with -fobjc-weak

Summary: -Warc-repeated-use-of-weak should produce the same warnings with 
-fobjc-weak as it does with -objc-arc. Also check for ObjCWeak along with 
ObjCAutoRefCount when recording the use of an evaluated weak variable. Add a 
-fobjc-weak run to the existing arc-repeated-weak test case and adapt it 
slightly to work in both modes.

Reviewers: rsmith, doug.gregor, jordan_rose, rjmccall

Reviewed By: rjmccall

Subscribers: arphaman, rjmccall, cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/test/SemaObjC/arc-repeated-weak.mm

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=299011&r1=299010&r2=299011&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Wed Mar 29 12:55:11 2017
@@ -1020,6 +1020,9 @@ public:
 return getQualifiers().hasStrongOrWeakObjCLifetime();
   }
 
+  // true when Type is objc's weak and weak is enabled but ARC isn't.
+  bool isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const;
+
   enum DestructionKind {
 DK_none,
 DK_cxx_destructor,

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=299011&r1=299010&r2=299011&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Wed Mar 29 12:55:11 2017
@@ -2148,7 +2148,11 @@ bool QualType::isTriviallyCopyableType(c
   return false;
 }
 
-
+bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
+  return !Context.getLangOpts().ObjCAutoRefCount &&
+ Context.getLangOpts().ObjCWeak &&
+ getObjCLifetime() != Qualifiers::OCL_Weak;
+}
 
 bool Type::isLiteralType(const ASTContext &Ctx) const {
   if (isDependentType())

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=299011&r1=299010&r2=299011&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar 29 12:55:11 2017
@@ -10167,7 +10167,8 @@ void Sema::AddInitializerToDecl(Decl *Re
 // we do not warn to warn spuriously when 'x' and 'y' are on separate
 // paths through the function. This should be revisited if
 // -Wrepeated-use-of-weak is made flow-sensitive.
-if (VDecl->getType().getObjCLifetime() == Qualifiers::OCL_Strong &&
+if ((VDecl->getType().getObjCLifetime() == Qualifiers::OCL_Strong ||
+ VDecl->getType().isNonWeakInMRRWithObjCWeak(Context)) &&
 !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak,
  Init->getLocStart()))
   getCurFunction()->markSafeWeakUse(Init);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=299011&r1=299010&r2=299011&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Mar 29 12:55:11 2017
@@ -704,8 +704,7 @@ ExprResult Sema::DefaultLvalueConversion
   
   // Loading a __weak object implicitly retains the value, so we need a 
cleanup to 
   // balance that.
-  if (getLangOpts().ObjCAutoRefCount &&
-  E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
+  if (E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
 Cleanup.setExprNeedsCleanups(true);
 
   ExprResult Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
@@ -2509,11 +2508,11 @@ Sema::LookupInObjCMethod(LookupResult &L
   ObjCIvarRefExpr(IV, IV->getUsageType(SelfExpr.get()->getType()), Loc,
   IV->getLocation(), SelfExpr.get(), true, true);
 
+  if (IV->getType().getObjCLifetime() == Qualifiers::OCL_Weak) {
+if (!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, Loc))
+  recordUseOfEvaluatedWeak(Result);
+  }
   if (getLangOpts().ObjCAutoRefCount) {
-if (IV->getType().getObjCLifetime() == Qualifiers::OCL_Weak) {
-  if (!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, Loc))
-recordUseOfEvaluatedWeak(Result);
-}
 if (CurContext->isClosure())
   Diag(Loc, diag::warn_implicitly_retains_self)
 << FixI

[PATCH] D29904: [OpenMP] Prevent emission of exception handling code when using OpenMP to offload to NVIDIA devices.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

LGTM. Please run `clang-format` before committing!




Comment at: test/OpenMP/target_parallel_no_exceptions.cpp:6-7
+
+#define SIZE 100
+#define EPS 1e-10
+

Not needed, please keep the test as small as possible


Repository:
  rL LLVM

https://reviews.llvm.org/D29904



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

r299012 - Unbreak windows bot.

[Graydon Hoare via cfe-commits]

Author: graydon
Date: Wed Mar 29 12:58:41 2017
New Revision: 299012

URL: http://llvm.org/viewvc/llvm-project?rev=299012&view=rev
Log:
Unbreak windows bot.

Modified:
cfe/trunk/test/PCH/emit-dependencies.c

Modified: cfe/trunk/test/PCH/emit-dependencies.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/emit-dependencies.c?rev=299012&r1=299011&r2=299012&view=diff
==
--- cfe/trunk/test/PCH/emit-dependencies.c (original)
+++ cfe/trunk/test/PCH/emit-dependencies.c Wed Mar 29 12:58:41 2017
@@ -1,7 +1,7 @@
 // RUN: rm -f %t.pch
 // RUN: %clang_cc1 -emit-pch -o %t.pch %S/Inputs/chain-decls1.h
 // RUN: %clang_cc1 -include-pch %t.pch -fsyntax-only -MT %s.o -dependency-file 
- %s | FileCheck %s
-// CHECK: Inputs/chain-decls1.h
+// CHECK: chain-decls1.h
 
 int main() {
   f();


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

Re: r298956 - Default enable the rtm feature only on skylake and later for now because Intel disabled the feature on some haswell and broadwell processors:

[Craig Topper via cfe-commits]

Thanks, Eric.

~Craig

On Wed, Mar 29, 2017 at 10:19 AM, Eric Christopher 
wrote:

> OK, I went ahead and  did this in the backend as well in:
>
> echristo@athyra ~/s/llvm> git svn dcommit
> Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
> M lib/Target/X86/X86.td
> Committed r298986
>
> -eric
>
> On Tue, Mar 28, 2017 at 5:49 PM Jim Grosbach  wrote:
>
>> SGTM
>>
>> Sent from my iPhone
>>
>> On Mar 28, 2017, at 5:25 PM, Eric Christopher  wrote:
>>
>>
>>
>> On Tue, Mar 28, 2017 at 4:31 PM Craig Topper 
>> wrote:
>>
>> So if you use -march=hsw the backend will think rtm is enabled, but clang
>> will block the intrinsics in the frontend?
>>
>>
>> Yeah. I've come to the conclusion that I think we should remove the
>> feature from the backend too. Using the option/feature in the frontend (or
>> during codegen of any sort) will turn on the instruction in the backend and
>> we should just rely on the option/feature.
>>
>>
>>
>> Not sure what you mean by split the haswell and broadwell cpu.
>>
>>
>> Into something similar to how the skylake cpu is configured in the front
>> end with different feature sets. I don't really think it's a good idea
>> here, but I thought I'd raise it.
>>
>> Thoughts?
>>
>> -eric
>>
>>
>>
>> ~Craig
>>
>> On Tue, Mar 28, 2017 at 4:18 PM, Eric Christopher 
>> wrote:
>>
>> Hi Craig, Quentin, Jim,
>>
>> Just bringing this patch to your attention here. I haven't turned it off
>> in the backend since some processors do support it and we want to allow the
>> code generation, with this change we simply make the user use -mrtm to get
>> the functionality for now.
>>
>> Question: Do we want to split the haswell and broadwell cpu to handle
>> this different feature set or just leave it as an optional "enable it
>> yourself" since it's an errata fix?
>>
>> I went ahead and committed with this question outstanding so that users
>> won't get illegal instructions if they happen to have the affected units.
>>
>> -eric
>>
>>
>> On Tue, Mar 28, 2017 at 4:15 PM Eric Christopher via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> Author: echristo
>> Date: Tue Mar 28 18:03:19 2017
>> New Revision: 298956
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=298956&view=rev
>> Log:
>> Default enable the rtm feature only on skylake and later for now because
>> Intel disabled the feature on some haswell and broadwell processors:
>>
>> http://www.intel.com/content/dam/www/public/us/en/
>> documents/specification-updates/core-m-processor-family-spec-update.pdf
>>
>> the -mrtm option will still work normally.
>>
>> Modified:
>> cfe/trunk/lib/Basic/Targets.cpp
>> cfe/trunk/test/Preprocessor/predefined-arch-macros.c
>>
>> Modified: cfe/trunk/lib/Basic/Targets.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
>> Targets.cpp?rev=298956&r1=298955&r2=298956&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Basic/Targets.cpp (original)
>> +++ cfe/trunk/lib/Basic/Targets.cpp Tue Mar 28 18:03:19 2017
>> @@ -3194,6 +3194,7 @@ bool X86TargetInfo::initFeatureMap(
>>  setFeatureEnabledImpl(Features, "mpx", true);
>>  setFeatureEnabledImpl(Features, "sgx", true);
>>  setFeatureEnabledImpl(Features, "clflushopt", true);
>> +setFeatureEnabledImpl(Features, "rtm", true);
>>  LLVM_FALLTHROUGH;
>>case CK_Broadwell:
>>  setFeatureEnabledImpl(Features, "rdseed", true);
>> @@ -3204,7 +3205,6 @@ bool X86TargetInfo::initFeatureMap(
>>  setFeatureEnabledImpl(Features, "lzcnt", true);
>>  setFeatureEnabledImpl(Features, "bmi", true);
>>  setFeatureEnabledImpl(Features, "bmi2", true);
>> -setFeatureEnabledImpl(Features, "rtm", true);
>>  setFeatureEnabledImpl(Features, "fma", true);
>>  setFeatureEnabledImpl(Features, "movbe", true);
>>  LLVM_FALLTHROUGH;
>>
>> Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>> Preprocessor/predefined-arch-macros.c?rev=298956&r1=298955&
>> r2=298956&view=diff
>> 
>> ==
>> --- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
>> +++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Tue Mar 28
>> 18:03:19 2017
>> @@ -525,7 +525,6 @@
>>  // CHECK_CORE_AVX2_M32: #define __PCLMUL__ 1
>>  // CHECK_CORE_AVX2_M32: #define __POPCNT__ 1
>>  // CHECK_CORE_AVX2_M32: #define __RDRND__ 1
>> -// CHECK_CORE_AVX2_M32: #define __RTM__ 1
>>  // CHECK_CORE_AVX2_M32: #define __SSE2__ 1
>>  // CHECK_CORE_AVX2_M32: #define __SSE3__ 1
>>  // CHECK_CORE_AVX2_M32: #define __SSE4_1__ 1
>> @@ -555,7 +554,6 @@
>>  // CHECK_CORE_AVX2_M64: #define __PCLMUL__ 1
>>  // CHECK_CORE_AVX2_M64: #define __POPCNT__ 1
>>  // CHECK_CORE_AVX2_M64: #define __RDRND__ 1
>> -// CHECK_CORE_AVX2_M64: #define __RTM__ 1
>>  // CHECK_CORE_AVX2_M64: #define __SSE2_MATH__ 1
>>  // CHECK_CORE_AVX2_M64: #define __SSE2__ 1
>>  //

[PATCH] D25157: [compiler-rt] [cmake] Respect COMPILER_RT_BUILD_* for libs, headers and tests

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: cmake/config-ix.cmake:438-441
 if (SANITIZER_COMMON_SUPPORTED_ARCH AND NOT LLVM_USE_SANITIZER AND
+(COMPILER_RT_BUILD_SANITIZERS OR COMPILER_RT_BUILD_XRAY) AND
 (OS_NAME MATCHES "Android|Darwin|Linux|FreeBSD" OR
 (OS_NAME MATCHES "Windows" AND (NOT MINGW AND NOT CYGWIN

What does this really leave in terms of targets which the sanitizer supports 
but doesn't have the common library?



Comment at: cmake/config-ix.cmake:562
   set(COMPILER_RT_HAS_XRAY FALSE)
 endif()

Seems like it might be a good time to hoist the OS checks and add a 
`COMPILER_RT_*_SUPPORTED` macro (e.g. `COMPILER_RT_XRAY_SUPPORTED`).



Comment at: lib/CMakeLists.txt:60
+# the following set is conditional to COMPILER_RT_BUILD_XRAY
+# (via COMPILER_RT_HAS_* in config-ix.cmake)
+compiler_rt_build_runtime(xray)

Im not sure I understand the comment.  This looks like it may prevent disabling 
X-ray?


https://reviews.llvm.org/D25157



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

r299014 - [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 13:09:02 2017
New Revision: 299014

URL: http://llvm.org/viewvc/llvm-project?rev=299014&view=rev
Log:
[Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

Summary: clang should produce the same errors Objective-C classes that cannot 
be assigned to weak pointers under both -fobjc-arc and -fobjc-weak. Check for 
ObjCWeak along with ObjCAutoRefCount when analyzing pointer conversions. Add an 
-fobjc-weak pass to the existing arc-unavailable-for-weakref test cases to 
verify the behavior is the same.

Reviewers: rsmith, doug.gregor, rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/test/SemaObjC/arc-unavailable-for-weakref.m
cfe/trunk/test/SemaObjCXX/arc-unavailable-for-weakref.mm

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=299014&r1=299013&r2=299014&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Wed Mar 29 13:09:02 2017
@@ -170,6 +170,11 @@ public:
   /// \brief Is this a libc/libm function that is no longer recognized as a
   /// builtin because a -fno-builtin-* option has been specified?
   bool isNoBuiltinFunc(StringRef Name) const;
+
+  /// \brief True if any ObjC types may have non-trivial lifetime qualifiers.
+  bool allowsNonTrivialObjCLifetimeQualifiers() const {
+return ObjCAutoRefCount || ObjCWeak;
+  }
 };
 
 /// \brief Floating point control options

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=299014&r1=299013&r2=299014&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 29 13:09:02 2017
@@ -9346,14 +9346,14 @@ public:
   enum ARCConversionResult { ACR_okay, ACR_unbridged, ACR_error };
 
   /// \brief Checks for invalid conversions and casts between
-  /// retainable pointers and other pointer kinds.
-  ARCConversionResult CheckObjCARCConversion(SourceRange castRange,
- QualType castType, Expr *&op,
- CheckedConversionKind CCK,
- bool Diagnose = true,
- bool DiagnoseCFAudited = false,
- BinaryOperatorKind Opc = 
BO_PtrMemD
- );
+  /// retainable pointers and other pointer kinds for ARC and Weak.
+  ARCConversionResult CheckObjCConversion(SourceRange castRange,
+  QualType castType, Expr *&op,
+  CheckedConversionKind CCK,
+  bool Diagnose = true,
+  bool DiagnoseCFAudited = false,
+  BinaryOperatorKind Opc = BO_PtrMemD
+  );
 
   Expr *stripARCUnbridgedCast(Expr *e);
   void diagnoseARCUnbridgedCast(Expr *e);

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=299014&r1=299013&r2=299014&view=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Wed Mar 29 13:09:02 2017
@@ -120,12 +120,12 @@ namespace {
   Self.CheckCastAlign(SrcExpr.get(), DestType, OpRange);
 }
 
-void checkObjCARCConversion(Sema::CheckedConversionKind CCK) {
-  assert(Self.getLangOpts().ObjCAutoRefCount);
+void checkObjCConversion(Sema::CheckedConversionKind CCK) {
+  assert(Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers());
 
   Expr *src = SrcExpr.get();
-  if (Self.CheckObjCARCConversion(OpRange, DestType, src, CCK) ==
-Sema::ACR_unbridged)
+  if (Self.CheckObjCConversion(OpRange, DestType, src, CCK) ==
+  Sema::ACR_unbridged)
 IsARCUnbridgedCast = true;
   SrcExpr = src;
 }
@@ -872,7 +872,7 @@ void CastOperation::CheckReinterpretCast
 SrcExpr = ExprError();
   } else if (tcr == TC_Success) {
 if (Self.getLangOpts().ObjCAutoRefCount)
-  checkObjCARCConversion(Sema::CCK_OtherCast);
+  checkObjCConversion(Sema::CCK_OtherCast);
 DiagnoseReinterpretUpDownCast(Self, SrcExpr.get(),

[PATCH] D29904: [OpenMP] Prevent emission of exception handling code when using OpenMP to offload to NVIDIA devices.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:2167-2172
+// Set the flag to prevent the implementation from emitting device 
exception
+// handling code for those requiring so.
+if (Opts.OpenMPIsDevice && T.isNVPTX()) {
+  Opts.Exceptions = 0;
+  Opts.CXXExceptions = 0;
+}

I'm not sure this is the right place for this code.



Comment at: test/OpenMP/target_parallel_no_exceptions.cpp:4
+
+#include 
+

You can't use `#include` directive in tests, declare `printf` function 
yourself, if you really need it.


Repository:
  rL LLVM

https://reviews.llvm.org/D29904



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

r299015 - [Objective-C] Miscellaneous -fobjc-weak Fixes

[Brian Kelley via cfe-commits]

Author: bkelley
Date: Wed Mar 29 13:16:38 2017
New Revision: 299015

URL: http://llvm.org/viewvc/llvm-project?rev=299015&view=rev
Log:
[Objective-C] Miscellaneous -fobjc-weak Fixes

Summary: After examining the remaining uses of LangOptions.ObjCAutoRefCount, 
found a some additional places to also check for ObjCWeak not covered by 
previous test cases. Added a test file to verify all the code paths that were 
changed.

Reviewers: rsmith, doug.gregor, rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/SemaObjCXX/objc-weak.mm
Modified:
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=299015&r1=299014&r2=299015&view=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Wed Mar 29 13:16:38 2017
@@ -871,7 +871,7 @@ void CastOperation::CheckReinterpretCast
 }
 SrcExpr = ExprError();
   } else if (tcr == TC_Success) {
-if (Self.getLangOpts().ObjCAutoRefCount)
+if (Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers())
   checkObjCConversion(Sema::CCK_OtherCast);
 DiagnoseReinterpretUpDownCast(Self, SrcExpr.get(), DestType, OpRange);
   }
@@ -935,7 +935,7 @@ void CastOperation::CheckStaticCast() {
   } else if (tcr == TC_Success) {
 if (Kind == CK_BitCast)
   checkCastAlign();
-if (Self.getLangOpts().ObjCAutoRefCount)
+if (Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers())
   checkObjCConversion(Sema::CCK_OtherCast);
   } else if (Kind == CK_BitCast) {
 checkCastAlign();

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=299015&r1=299014&r2=299015&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar 29 13:16:38 2017
@@ -14468,7 +14468,7 @@ void Sema::ActOnFields(Scope *S, SourceL
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ARCErrReported = false;
+  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
@@ -14603,16 +14603,16 @@ void Sema::ActOnFields(Scope *S, SourceL
 << FixItHint::CreateInsertion(FD->getLocation(), "*");
   QualType T = Context.getObjCObjectPointerType(FD->getType());
   FD->setType(T);
-} else if (getLangOpts().ObjCAutoRefCount && Record && !ARCErrReported &&
+} else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
+   Record && !ObjCFieldLifetimeErrReported &&
(!getLangOpts().CPlusPlus || Record->isUnion())) {
-  // It's an error in ARC if a field has lifetime.
+  // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
   // FIXME: that's really not sufficient; we need to make the type
   // itself invalid to, say, initialize or copy.
   QualType T = FD->getType();
-  Qualifiers::ObjCLifetime lifetime = T.getObjCLifetime();
-  if (lifetime && lifetime != Qualifiers::OCL_ExplicitNone) {
+  if (T.hasNonTrivialObjCLifetime()) {
 SourceLocation loc = FD->getLocation();
 if (getSourceManager().isInSystemHeader(loc)) {
   if (!FD->hasAttr()) {
@@ -14623,7 +14623,7 @@ void Sema::ActOnFields(Scope *S, SourceL
   Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
 << T->isBlockPointerType() << Record->getTagKind();
 }
-ARCErrReported = true;
+ObjCFieldLifetimeErrReported = true;
   }
 } else if (getLangOpts().ObjC1 &&
getLangOpts().getGC() != LangOptions::NonGC &&

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=299015&r1=299014&r2=299015&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Mar 29 13:16:38 2017
@@ -7145,8 +7145,7 @@ static bool checkTrivialClassMembers(Sem
 //   [...] nontrivally ownership-qualified types are [...] not trivially
 //   default constructible, copy constructible, move constructible, copy
 //   assignable, move assignable, or destructible [...]
-if (S.getLangOpts().ObjCAutoRefCount &&
-FieldType.hasNonTrivialObjCLifetime()) {
+if (FieldType.hasNonTrivialObjCLifetime()) {
   if (Diagnose)
 S.Diag(FI->getLocatio

[PATCH] D31440: PR32382: Adapt to LLVM changes in DIExpression.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D31440#713308, @dblaikie wrote:

> I'm a bit confused - the alloca was only emitted at -O0, by the looks of it. 
> Presumably it's pessimizing in some way at higher optimization levels? Or is 
> that not the case?


I think it is really working around the odd behavior of LLVM here. What gives 
it away is the we key the addition of the extra DW_OP_deref on whether it is an 
alloca or not. But note that this is not how dbg.declare works: 
dbg.declare(%alloca, !DIExpression()) is (kind of) equivalent to 
dbg.value(%alloca, !DIExpression(DW_OP_deref)). So we are using the presence of 
the alloca as a proxy for how the backend happens to compile the 
DwarfExpression here and work around its idiosyncrasy by emitting an extra 
DW_OP_deref.

> Also, it looks like this change lost the "if > gmlt" test, so might cause 
> variable declarations (& thus types) to leak into GMLT, which is undesirable.

Oh.. that was unintentional collateral damage. Thanks for noticing!


https://reviews.llvm.org/D31440



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

[PATCH] D25157: [compiler-rt] [cmake] Respect COMPILER_RT_BUILD_* for libs, headers and tests

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

mgorny added inline comments.



Comment at: cmake/config-ix.cmake:438-441
 if (SANITIZER_COMMON_SUPPORTED_ARCH AND NOT LLVM_USE_SANITIZER AND
+(COMPILER_RT_BUILD_SANITIZERS OR COMPILER_RT_BUILD_XRAY) AND
 (OS_NAME MATCHES "Android|Darwin|Linux|FreeBSD" OR
 (OS_NAME MATCHES "Windows" AND (NOT MINGW AND NOT CYGWIN

compnerd wrote:
> What does this really leave in terms of targets which the sanitizer supports 
> but doesn't have the common library?
I'm not sure if I understand your question correctly. If it's about  the 
platform logic, I don't know what it is about and I think it's a bit out of 
scope of this change. My goal is to merely disable sanitizer-related stuff when 
sanitizers are not built.



Comment at: cmake/config-ix.cmake:562
   set(COMPILER_RT_HAS_XRAY FALSE)
 endif()

compnerd wrote:
> Seems like it might be a good time to hoist the OS checks and add a 
> `COMPILER_RT_*_SUPPORTED` macro (e.g. `COMPILER_RT_XRAY_SUPPORTED`).
Sounds like material for a separate patch to me.



Comment at: lib/CMakeLists.txt:60
+# the following set is conditional to COMPILER_RT_BUILD_XRAY
+# (via COMPILER_RT_HAS_* in config-ix.cmake)
+compiler_rt_build_runtime(xray)

compnerd wrote:
> Im not sure I understand the comment.  This looks like it may prevent 
> disabling X-ray?
The comment was supposed to indicate that we don't need 
`if(COMPILER_RT_BUILD_XRAY)` here because it is already included in 
COMPILER_RT_HAS_... value.


https://reviews.llvm.org/D25157



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

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:119
+   DisabledMove = false;
+  for (const auto *OtherCtor : Ctor->getParent()->ctors()) {
+if (OtherCtor->isCopyConstructor()) {

leanil wrote:
> This is the most precise way to formulate the warning message I could come up 
> with.
> The condition for excluding either "copy" or "move" from the warning is to 
> find only disabled instances of the constructor type, and there must be at 
> least one, otherwise the compiler generated constructor (which is not present 
> in this enumeration) can be hidden.
How about:
```
if (OtherCtor->isDeleted() || OtherCtor->getAccess() == AS_private)
  (OtherCtor->isCopyConstructor() ? DisabledCopy : DisabledMove) = true;
else
  (OtherCtor->isCopyConstructor() ? EnabledCopy : EnabledMove) = true;
```



Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:136
+   NoMove = DisabledMove && !EnabledMove;
+  if (NoCopy && NoMove) {
+return;

Can elide the braces.



Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:140
+  diag(Ctor->getLocation(), "constructor accepting a forwarding reference can "
+"hide the %select{copy|move|copy and the "
+"move}0 constructor%s1")

copy and the move -> copy and move



Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:142
+"move}0 constructor%s1")
+  << 2 - NoCopy - 2 * NoMove << NoCopy + NoMove;
+  for (const auto *OtherCtor : Ctor->getParent()->ctors()) {

The math is correct, but really makes me scratch my head to try to puzzle 
through it. Why not something along the lines of `Copy && Move ? 2 : (Copy ? 0 
: 1)`


Repository:
  rL LLVM

https://reviews.llvm.org/D30547



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

[PATCH] D29599: Clang Changes for alloc_align

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1230
+  let Spellings = [GCC<"alloc_align">];
+  let Subjects = SubjectList<[ Function]>;
+  let Args = [IntArgument<"ParamIndex">];

There's a spurious space between [ and Function.

If we want to behave like GCC, then your subject list is fine. If we want to 
tighten up what we allow rather than silently accept the attribute and do 
nothing with it, this should probably be HasFunctionProto because the attribute 
doesn't seem to make sense on an unprototyped function.



Comment at: include/clang/Basic/Attr.td:1224
+def AllocAlign : InheritableAttr {
+  let Spellings = [ GCC<"alloc_align"> ];
+  let Subjects = SubjectList<[Function]>;

erichkeane wrote:
> aaron.ballman wrote:
> > Extra spaces in the declaration that do not match the style of the rest of 
> > the file (same happens below).
> Strangely, this is ClangFormat at work :/
Ah, I guess clang-format doesn't quite understand .td files.



Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter, starting
+with 1.

aaron.ballman wrote:
> I would split the "starting with 1" off into its own (full) sentence for 
> clarity purposes. Also, please spell out one instead of 1.
> 
> You may also want to clarify how member functions do/do not impact this index.
This does not appear to have been handled?



Comment at: lib/CodeGen/CGCall.cpp:4374
+} else if (const auto *AA = TargetDecl->getAttr()) {
+  llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+  EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);

erichkeane wrote:
> aaron.ballman wrote:
> > Instead of hoping all of the callers of `getParamIndex()` will remember 
> > that this is a weird one-based thing, why not give the semantic attribute 
> > the correct index when attaching the attribute to the declaration?
> I played with this for a while, and the difficulty is that AddAllocAlignAttr 
> requires the 1-based index, since it needs to error based on that number.  
> Additionally, decrementing the value BEFORE that function becomes difficult, 
> since it is an Expr object at that time (which would get messy in the 
> template case).
> 
> I cannot alter it in the AddAllocAlignAttr function, since that function 
> actually gets called TWICE in the template instantiation case, so I'd likely 
> need some sort of flag that told whether to treat it as decremented or not.  
> In general, this gets really ugly really quickly.
> 
> Basically, I don't see a good place to decrement the value anywhere without 
> causing a much more significant change.  
Thank you for the explanation, that makes sense to me.



Comment at: lib/Sema/SemaDeclAttr.cpp:1599
+  int IndexVal;
+  if (!checkPositiveIntArgument(*this, TmpAttr, ParamExpr, IndexVal,
+/*Index=*/1))

erichkeane wrote:
> aaron.ballman wrote:
> > It seems strange to me that you check that it's a positive integer argument 
> > before checking the param is an integer type.
> > 
> > Why not use `checkFunctionOrMethodParameterIndex()`?
> I'm unaware of checkFunctionOrMethodParameterIndex, there are a ton of odd 
> free fucntions around here, I just copied from some of the surrounding 
> functions.
> 
> That said, these two poorly named functions are actually checking 2 different 
> pieces of data.  So in the case of:
> void func(int foo) __attribute__((alloc_align(1));
> 
> The "checkPositiveIntArgument" checks to ensure that '1' is a positive 
> integer.  "checkParamIsIntegerType" checks that "foo" (the corresponding 
> parameter) is an integer, and that '1' is in range.
Ah, I see (and yes, that function name is rather ambiguous). I think you should 
be using `checkFunctionOrMethodParameterIndex()` in place of 
`checkPositiveIntArgument()` and leave in the `checkParamIsIntegerType()`.


https://reviews.llvm.org/D29599



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

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93401.
zhizhouy marked 2 inline comments as done.
zhizhouy edited the summary of this revision.
zhizhouy added a comment.

Added two more testcases, one is options with both grecord-gcc-switches and 
gno-record-gcc-switches; the other one is testing if "-o -" is omitted 
correctly.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,16 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +202,15 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto &Arg : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,16 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +202,15 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
  

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added inline comments.



Comment at: test/Driver/debug-options.c:201-202
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//

george.burgess.iv wrote:
> echristo wrote:
> > This seems a little light on the testing, would you mind adding some more 
> > interesting lines here? (Also, -grecord-gcc-switches seems like an option 
> > we might want to ignore for this?)
> re-pasting my comment, since it looks like it got dropped when we updated 
> diffs:
> 
> nit: since `RenderAsInput` args (`-Xlinker`, `-o`, ...) have cases where 
> they're printed without their arg, can we check that at least `-o -` is 
> printed sanely, as well?
How about adding things like "-O3" or "-ffunction-sections" or something? Just 
useful options that we'd expect to see.



Comment at: test/Driver/debug-options.c:209
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//

Typo.


https://reviews.llvm.org/D30760



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

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93403.
zhizhouy marked 2 inline comments as done.
zhizhouy added a comment.

Added testcase for recording other useful options.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,18 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +204,17 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -grecord-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
+// GRECORD_OPT: -### -c -O3 -ffunction-sections -grecord-gcc-switches
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto &Arg : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,18 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +204,17 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -grecord-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
+// GRECORD_OPT: -### -c -O3 -ffunction-sections -grecord-gcc-switches
+//
 // GIGNORE-NOT: "argument unused d

[PATCH] D31167: Use FPContractModeKind universally

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor comment nit, LGTM




Comment at: include/clang/Basic/LangOptions.h:92
+  enum FPContractModeKind {
+FPC_Off,// Form fused FP ops only where result will not be 
affected.
+FPC_On, // Form fused FP ops according to FP_CONTRACT rules.

I think you mean effected rather than affected.


https://reviews.llvm.org/D31167



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

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

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

Sounds good. Do you have commit access?


https://reviews.llvm.org/D30760



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

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via cfe-commits]

Not really.

Would you please help me commit it? Also there is another diff for LLVM
part of this bug.

Thanks.

On Wed, Mar 29, 2017 at 1:16 PM, Eric Christopher via Phabricator <
revi...@reviews.llvm.org> wrote:

> echristo accepted this revision.
> echristo added a comment.
> This revision is now accepted and ready to land.
>
> Sounds good. Do you have commit access?
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter, starting
+with 1.

aaron.ballman wrote:
> aaron.ballman wrote:
> > I would split the "starting with 1" off into its own (full) sentence for 
> > clarity purposes. Also, please spell out one instead of 1.
> > 
> > You may also want to clarify how member functions do/do not impact this 
> > index.
> This does not appear to have been handled?
Ah, I missed that you wanted it in its own sentence, not just its own line.  
I'll expand on this text in the next patch.


https://reviews.llvm.org/D29599



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

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

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

Made the changes as requested.  checkFunctionOrMethodParameterIndex corrects 
for 1->0 index and implicit this, which requires undoing, otherwise templates 
create a big hassle.  Additionally, please note the AttrDocs changes, I think 
I've got it acceptable but would love a second english-smith.


https://reviews.llvm.org/D29599

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/CodeGen/alloc-align-attr.c
  test/Sema/alloc-align-attr.c
  test/SemaCXX/alloc-align-attr.cpp

Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4348,6 +4348,9 @@
   llvm::ConstantInt *AlignmentCI = cast(Alignment);
   EmitAlignmentAssumption(Ret.getScalarVal(), AlignmentCI->getZExtValue(),
   OffsetValue);
+} else if (const auto *AA = TargetDecl->getAttr()) {
+  llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+  EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);
 }
   }
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2465,6 +2465,12 @@
   PeepholeProtection protectFromPeepholes(RValue rvalue);
   void unprotectFromPeepholes(PeepholeProtection protection);
 
+  void EmitAlignmentAssumption(llvm::Value *PtrValue, llvm::Value *Alignment,
+   llvm::Value *OffsetValue = nullptr) {
+Builder.CreateAlignmentAssumption(CGM.getDataLayout(), PtrValue, Alignment,
+  OffsetValue);
+  }
+
   //======//
   // Statement Emission
   //======//
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -168,6 +168,16 @@
 Aligned->getSpellingListIndex());
 }
 
+static void instantiateDependentAllocAlignAttr(
+Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
+const AllocAlignAttr *Align, Decl *New) {
+  Expr *Param = IntegerLiteral::Create(
+  S.getASTContext(), llvm::APInt(64, Align->getParamIndex()),
+  S.getASTContext().UnsignedLongLongTy, Align->getLocation());
+  S.AddAllocAlignAttr(Align->getLocation(), New, Param,
+  Align->getSpellingListIndex());
+}
+
 static Expr *instantiateDependentFunctionAttrCondition(
 Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
 const Attr *A, Expr *OldCond, const Decl *Tmpl, FunctionDecl *New) {
@@ -380,6 +390,12 @@
   continue;
 }
 
+if (const auto *AllocAlign = dyn_cast(TmplAttr)) {
+  instantiateDependentAllocAlignAttr(*this, TemplateArgs, AllocAlign, New);
+  continue;
+}
+
+
 if (const auto *EnableIf = dyn_cast(TmplAttr)) {
   instantiateDependentEnableIfAttr(*this, TemplateArgs, EnableIf, Tmpl,
cast(New));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -218,21 +218,45 @@
std::greater());
 }
 
+/// \brief A helper function to provide Attribute Location for the Attr types
+/// AND the AttributeList.
+template 
+static typename std::enable_if::value,
+   SourceLocation>::type
+getAttrLoc(const AttrInfo &Attr) {
+  return Attr.getLocation();
+}
+static SourceLocation getAttrLoc(const clang::AttributeList &Attr) {
+  return Attr.getLoc();
+}
+
+/// \brief A helper function to provide Attribute Name for the Attr types
+/// AND the AttributeList.
+template 
+static typename std::enable_if::value,
+   const AttrInfo *>::type
+getAttrName(const AttrInfo &Attr) {
+  return &Attr;
+}
+const IdentifierInfo *getAttrName(const clang::AttributeList &Attr) {
+  return Attr.getName();
+}
+
 /// \brief If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
-static bool checkUInt32Argument(Sema &S, const AttributeList &Attr,
-const Expr *Expr, uint32_t &Val,
-unsigned Idx = UINT_MAX) {
+template
+static bool checkUInt32Argument(Sema &S, const AttrInfo& Attr, const Expr *Expr,
+uint32_t &Val, unsigned Idx = UINT_MAX) {
   llvm::APSInt I(32);
   if (Expr->is

[PATCH] D31167: Use FPContractModeKind universally

[Adam Nemet via Phabricator via cfe-commits]

anemet added inline comments.



Comment at: include/clang/Basic/LangOptions.h:92
+  enum FPContractModeKind {
+FPC_Off,// Form fused FP ops only where result will not be 
affected.
+FPC_On, // Form fused FP ops according to FP_CONTRACT rules.

aaron.ballman wrote:
> I think you mean effected rather than affected.
I think the verb is affect; effect is the noun.  Quick grep confirms:

/org/llvm$ git grep affected | wc -l
 109
/org/llvm$ git grep effected | wc -l
   2



https://reviews.llvm.org/D31167



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

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: lib/CodeGen/CGCoroutine.cpp:225
+  void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+  }

This will be called twice: once for a normal exit and once for exceptional 
exit. In general, double emitting a `Stmt*` is not safe, since it might contain 
a VarDecl or a LabelDecl, but this usage is safe because 
`SubStmtBuilder::makeNewAndDeleteExpr()` builds two calls that can't declare 
anything. That is *definitely* worth a comment. :)


https://reviews.llvm.org/D31460



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

[PATCH] D31167: Use FPContractModeKind universally

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/LangOptions.h:92
+  enum FPContractModeKind {
+FPC_Off,// Form fused FP ops only where result will not be 
affected.
+FPC_On, // Form fused FP ops according to FP_CONTRACT rules.

anemet wrote:
> aaron.ballman wrote:
> > I think you mean effected rather than affected.
> I think the verb is affect; effect is the noun.  Quick grep confirms:
> 
> /org/llvm$ git grep affected | wc -l
>  109
> /org/llvm$ git grep effected | wc -l
>2
> 
I think I was thrown by "form" being the verb in that sentence, but you are 
correct. :-)


https://reviews.llvm.org/D31167



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

r299027 - Use FPContractModeKind universally

[Adam Nemet via cfe-commits]

Author: anemet
Date: Wed Mar 29 15:39:49 2017
New Revision: 299027

URL: http://llvm.org/viewvc/llvm-project?rev=299027&view=rev
Log:
Use FPContractModeKind universally

FPContractModeKind is the codegen option flag which is already ternary (off,
on, fast).  This makes it universally the type for the contractable info
across the front-end:

* In FPOptions (i.e. in the Sema + in the expression nodes).
* In LangOpts::DefaultFPContractMode which is the option that initializes
FPOptions in the Sema.

Another way to look at this change is that before fp-contractable on/off were
the only states handled to the front-end:
 * For "on", FMA folding was performed by  the front-end
 * For "fast", we simply forwarded the flag to TargetOptions to handle it in
 LLVM

Now off/on/fast are all exposed because for fast we will generate
fast-math-flags during CodeGen.

This is toward moving fp-contraction=fast from an LLVM TargetOption to a
FastMathFlag in order to fix PR25721.

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=299027&r1=299026&r2=299027&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Wed Mar 29 15:39:49 2017
@@ -2920,7 +2920,7 @@ private:
 
   // This is only meaningful for operations on floating point types and 0
   // otherwise.
-  unsigned FPFeatures : 1;
+  unsigned FPFeatures : 2;
   SourceLocation OpLoc;
 
   enum { LHS, RHS, END_EXPR };
@@ -3078,8 +3078,8 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractable() const {
-return FPOptions(FPFeatures).isFPContractable();
+  bool isFPContractableWithinStatement() const {
+return FPOptions(FPFeatures).allowFPContractWithinStatement();
   }
 
 protected:

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=299027&r1=299026&r2=299027&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Wed Mar 29 15:39:49 2017
@@ -117,7 +117,9 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractable() const { return FPFeatures.isFPContractable(); }
+  bool isFPContractableWithinStatement() const {
+return FPFeatures.allowFPContractWithinStatement();
+  }
 
   friend class ASTStmtReader;
   friend class ASTStmtWriter;

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=299027&r1=299026&r2=299027&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Mar 29 15:39:49 2017
@@ -216,7 +216,8 @@ BENIGN_LANGOPT(DebuggerObjCLiteral , 1,
 BENIGN_LANGOPT(SpellChecking , 1, 1, "spell-checking")
 LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating 
point constants as single precision constants")
 LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math")
-LANGOPT(DefaultFPContract , 1, 0, "FP_CONTRACT")
+/// \brief FP_CONTRACT mode (on/off/fast).
+ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP 
contraction type")
 LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
 LANGOPT(HexagonQdsp6Compat , 1, 0, "hexagon-qdsp6 backward compatibility")
 LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting")

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=299027&r1=299026&r2=299027&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Wed Mar 29 15:39:49 2017
@@ -88,6 +88,12 @@ public:
 MSVC2015 = 19
   };
 
+  enum FPContractModeKind {
+FPC_Off,// Form fused FP ops only where result will not be 
affected.
+FPC_On, // Form fused FP ops according to FP_CONTRACT rules.
+FPC_F

[PATCH] D31167: Use FPContractModeKind universally

[Adam Nemet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299027: Use FPContractModeKind universally (authored by 
anemet).

Changed prior to commit:
  https://reviews.llvm.org/D31167?vs=92423&id=93406#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31167

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Basic/LangOptions.h
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/include/clang/Frontend/CodeGenOptions.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Sema/SemaAttr.cpp

Index: cfe/trunk/lib/Sema/SemaAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaAttr.cpp
+++ cfe/trunk/lib/Sema/SemaAttr.cpp
@@ -450,13 +450,16 @@
 void Sema::ActOnPragmaFPContract(tok::OnOffSwitch OOS) {
   switch (OOS) {
   case tok::OOS_ON:
-FPFeatures.setFPContractable(true);
+FPFeatures.setAllowFPContractWithinStatement();
 break;
   case tok::OOS_OFF:
-FPFeatures.setFPContractable(false);
+FPFeatures.setDisallowFPContract();
 break;
   case tok::OOS_DEFAULT:
-FPFeatures.setFPContractable(getLangOpts().DefaultFPContract);
+if (getLangOpts().getDefaultFPContractMode() == LangOptions::FPC_On)
+  FPFeatures.setAllowFPContractWithinStatement();
+else
+  FPFeatures.setDisallowFPContract();
 break;
   }
 }
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -816,18 +816,6 @@
 }
   }
 
-  if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
-StringRef Val = A->getValue();
-if (Val == "fast")
-  Opts.setFPContractMode(CodeGenOptions::FPC_Fast);
-else if (Val == "on")
-  Opts.setFPContractMode(CodeGenOptions::FPC_On);
-else if (Val == "off")
-  Opts.setFPContractMode(CodeGenOptions::FPC_Off);
-else
-  Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
-  }
-
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
 StringRef Val = A->getValue();
 if (Val == "ieee")
@@ -1647,7 +1635,7 @@
 Opts.AltiVec = 0;
 Opts.ZVector = 0;
 Opts.LaxVectorConversions = 0;
-Opts.DefaultFPContract = 1;
+Opts.setDefaultFPContractMode(LangOptions::FPC_On);
 Opts.NativeHalfType = 1;
 Opts.NativeHalfArgsAndReturns = 1;
 // Include default header file for OpenCL.
@@ -1658,6 +1646,9 @@
 
   Opts.CUDA = IK == IK_CUDA || IK == IK_PreprocessedCuda ||
   LangStd == LangStandard::lang_cuda;
+  if (Opts.CUDA)
+// Set default FP_CONTRACT to FAST.
+Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
 
   Opts.RenderScript = IK == IK_RenderScript;
   if (Opts.RenderScript) {
@@ -2282,6 +2273,18 @@
   Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
   Args.hasArg(OPT_cl_fast_relaxed_math);
 
+  if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
+StringRef Val = A->getValue();
+if (Val == "fast")
+  Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
+else if (Val == "on")
+  Opts.setDefaultFPContractMode(LangOptions::FPC_On);
+else if (Val == "off")
+  Opts.setDefaultFPContractMode(LangOptions::FPC_Off);
+else
+  Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
+  }
+
   Opts.RetainCommentsFromSystemHeaders =
   Args.hasArg(OPT_fretain_comments_from_system_headers);
 
@@ -2536,10 +2539,6 @@
 // triple used for host compilation.
 if (LangOpts.CUDAIsDevice)
   Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple;
-
-// Set default FP_CONTRACT to FAST.
-if (!Args.hasArg(OPT_ffp_contract))
-  Res.getCodeGenOpts().setFPContractMode(CodeGenOptions::FPC_Fast);
   }
 
   // FIXME: Override value name discarding when asan or msan is used because the
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -2672,12 +2672,7 @@
  "Only fadd/fsub can be the root of an fmuladd.");
 
   // Check whether this op is marked as fusable.
-  if (!op.FPFeatures.isFPContractable())
-return nullptr;
-
-  // Check whether -ffp-contract=on. (If -ffp-contract=off/fast, fusing is
-  // either disabled, or handled entirely by the LLVM backend).
-  if (CGF.CGM.getCodeGenOpts().getFPContractMode() != CodeGenOptions::FPC_On)
+  if (!op.FPFeatures.allowFPContractWithinStatement())
 return nullptr;
 
   // We have a potentially fusable op. Look for a mul on one of the operands.
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
=

[PATCH] D29599: Clang Changes for alloc_align

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter.  The
+alignment parameter is one-indexed.  In the case of member functions, the

Use single spacing instead of double spacing.



Comment at: include/clang/Basic/AttrDocs.td:253
+pointer type) has an alignment specified by the indicated parameter.  The
+alignment parameter is one-indexed.  In the case of member functions, the
+implicit ``this`` parameter is considered as as the first function parameter.

s/one-index/one-based?

member functions -> nonstatic member functions



Comment at: include/clang/Basic/AttrDocs.td:254
+alignment parameter is one-indexed.  In the case of member functions, the
+implicit ``this`` parameter is considered as as the first function parameter.
+

is considered as as the first -> is the first



Comment at: include/clang/Basic/AttrDocs.td:258
+
+  // The returned pointer has the alignment specified by the first parameter
+  void *a(size_t align) __attribute__((alloc_align(1)));

Missing full stop.



Comment at: include/clang/Basic/AttrDocs.td:261
+
+  // The returned pointer has the alignment specified by the second parameter
+  void *b(void* v, size_t align) __attribute__((alloc_align(2)));

Same here.



Comment at: include/clang/Basic/AttrDocs.td:262
+  // The returned pointer has the alignment specified by the second parameter
+  void *b(void* v, size_t align) __attribute__((alloc_align(2)));
+

Pointer should bind to `v` (same below).



Comment at: lib/Sema/SemaDeclAttr.cpp:809
+/// an integral type. Will emit appropriate diagnostics if this returns
+/// false.
+///

This can be hoisted up to the previous line.



Comment at: lib/Sema/SemaDeclAttr.cpp:1530
+   const AttributeList &Attr) {
+  Expr *E = Attr.getArgAsExpr(0);
+  S.AddAllocAlignAttr(Attr.getRange(), D, E, 

There's no real need for `E`.



Comment at: lib/Sema/SemaDeclAttr.cpp:1532
+  S.AddAllocAlignAttr(Attr.getRange(), D, E, 
+   Attr.getAttributeSpellingListIndex());
+}

This formatting looks off (the args should align).



Comment at: lib/Sema/SemaDeclAttr.cpp:1608-1612
+  IndexVal += 1 + isInstanceMethod(FuncDecl);
+
+  if (!checkParamIsIntegerType(*this, FuncDecl, TmpAttr, ParamExpr, IndexVal,
+   /*AttrArgNo=*/0, /*AllowDependentType=*/true))
+return;

Hmmm, I think this might be a bit more clean as:
```
QualType Ty = getFunctionOrMethodParamType(D, IndexVal);
if (!Ty->isIntegralType(Context)) {
  Diag(ParamExpr->getLocStart(), diag::err_attribute_integers_only) << TmpAttr 
<<
 FuncDecl->getParamDecl(IndexVal)->getSourceRange();
  return;
}

// We cannot use the Idx returned from checkFunctionOrMethodParameterIndex
// because that has corrected for the implicit this parameter, and is zero-
// based.  The attribute expects what the user wrote explicitly.
llvm::APSInt Val;
ParamExpr->EvaluateAsInt(Val, S.Context);

// Use Val.getZExtValue() when you need what the user wrote.
```
This matches more closely with how other attributes handle the situation where 
the Attr object needs what the user wrote (like format_arg). I hadn't noticed 
that checkParamIsIntegerType() turns around and calls 
checkFunctionOrMethodParameterIndex() again, and this would simplify your patch 
a bit.

What do you think?


https://reviews.llvm.org/D29599



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

r299029 - Revert "Use FPContractModeKind universally"

[Adam Nemet via cfe-commits]

Author: anemet
Date: Wed Mar 29 16:24:19 2017
New Revision: 299029

URL: http://llvm.org/viewvc/llvm-project?rev=299029&view=rev
Log:
Revert "Use FPContractModeKind universally"

This reverts commit r299027.

It's causing a test failure in clang's CodeGenCUDE/fp-contract.cu

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=299029&r1=299028&r2=299029&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Wed Mar 29 16:24:19 2017
@@ -2920,7 +2920,7 @@ private:
 
   // This is only meaningful for operations on floating point types and 0
   // otherwise.
-  unsigned FPFeatures : 2;
+  unsigned FPFeatures : 1;
   SourceLocation OpLoc;
 
   enum { LHS, RHS, END_EXPR };
@@ -3078,8 +3078,8 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractableWithinStatement() const {
-return FPOptions(FPFeatures).allowFPContractWithinStatement();
+  bool isFPContractable() const {
+return FPOptions(FPFeatures).isFPContractable();
   }
 
 protected:

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=299029&r1=299028&r2=299029&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Wed Mar 29 16:24:19 2017
@@ -117,9 +117,7 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractableWithinStatement() const {
-return FPFeatures.allowFPContractWithinStatement();
-  }
+  bool isFPContractable() const { return FPFeatures.isFPContractable(); }
 
   friend class ASTStmtReader;
   friend class ASTStmtWriter;

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=299029&r1=299028&r2=299029&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Mar 29 16:24:19 2017
@@ -216,8 +216,7 @@ BENIGN_LANGOPT(DebuggerObjCLiteral , 1,
 BENIGN_LANGOPT(SpellChecking , 1, 1, "spell-checking")
 LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating 
point constants as single precision constants")
 LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math")
-/// \brief FP_CONTRACT mode (on/off/fast).
-ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP 
contraction type")
+LANGOPT(DefaultFPContract , 1, 0, "FP_CONTRACT")
 LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
 LANGOPT(HexagonQdsp6Compat , 1, 0, "hexagon-qdsp6 backward compatibility")
 LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting")

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=299029&r1=299028&r2=299029&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Wed Mar 29 16:24:19 2017
@@ -88,12 +88,6 @@ public:
 MSVC2015 = 19
   };
 
-  enum FPContractModeKind {
-FPC_Off,// Form fused FP ops only where result will not be 
affected.
-FPC_On, // Form fused FP ops according to FP_CONTRACT rules.
-FPC_Fast// Aggressively fuse FP ops (E.g. FMA).
-  };
-
 public:
   /// \brief Set of enabled sanitizers.
   SanitizerSet Sanitize;
@@ -186,35 +180,22 @@ public:
 /// \brief Floating point control options
 class FPOptions {
 public:
-  FPOptions() : fp_contract(LangOptions::FPC_Off) {}
+  FPOptions() : fp_contract(0) {}
 
-  // Used for serializing.
-  explicit FPOptions(unsigned I)
-  : fp_contract(static_cast(I)) {}
+  explicit FPOptions(unsigned I) : fp_contract(I) {}
 
   explicit FPOptions(const LangOptions &LangOpts)
-  : fp_contract(LangOpts.getDefaultFPContractMode()) {}
+  : fp_contract(LangOpts.DefaultFPContract) {}
 
-  bool allowFPContractWithinStatement() const {
-return fp_contract == LangOptions::FPC_On;
-  }
-

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDeclAttr.cpp:1608-1612
+  IndexVal += 1 + isInstanceMethod(FuncDecl);
+
+  if (!checkParamIsIntegerType(*this, FuncDecl, TmpAttr, ParamExpr, IndexVal,
+   /*AttrArgNo=*/0, /*AllowDependentType=*/true))
+return;

aaron.ballman wrote:
> Hmmm, I think this might be a bit more clean as:
> ```
> QualType Ty = getFunctionOrMethodParamType(D, IndexVal);
> if (!Ty->isIntegralType(Context)) {
>   Diag(ParamExpr->getLocStart(), diag::err_attribute_integers_only) << 
> TmpAttr <<
>  FuncDecl->getParamDecl(IndexVal)->getSourceRange();
>   return;
> }
> 
> // We cannot use the Idx returned from checkFunctionOrMethodParameterIndex
> // because that has corrected for the implicit this parameter, and is zero-
> // based.  The attribute expects what the user wrote explicitly.
> llvm::APSInt Val;
> ParamExpr->EvaluateAsInt(Val, S.Context);
> 
> // Use Val.getZExtValue() when you need what the user wrote.
> ```
> This matches more closely with how other attributes handle the situation 
> where the Attr object needs what the user wrote (like format_arg). I hadn't 
> noticed that checkParamIsIntegerType() turns around and calls 
> checkFunctionOrMethodParameterIndex() again, and this would simplify your 
> patch a bit.
> 
> What do you think?
Looks better, I hadn't realized checkParamIsIntegerType was redoing work either.

I had to make a pair of small changes to this (including omitting DependentType 
from the integral type check), but it otherwise works nicely.  Thanks!


https://reviews.llvm.org/D29599



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

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 93409.

https://reviews.llvm.org/D29599

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/CodeGen/alloc-align-attr.c
  test/Sema/alloc-align-attr.c
  test/SemaCXX/alloc-align-attr.cpp

Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4348,6 +4348,9 @@
   llvm::ConstantInt *AlignmentCI = cast(Alignment);
   EmitAlignmentAssumption(Ret.getScalarVal(), AlignmentCI->getZExtValue(),
   OffsetValue);
+} else if (const auto *AA = TargetDecl->getAttr()) {
+  llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+  EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);
 }
   }
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2465,6 +2465,12 @@
   PeepholeProtection protectFromPeepholes(RValue rvalue);
   void unprotectFromPeepholes(PeepholeProtection protection);
 
+  void EmitAlignmentAssumption(llvm::Value *PtrValue, llvm::Value *Alignment,
+   llvm::Value *OffsetValue = nullptr) {
+Builder.CreateAlignmentAssumption(CGM.getDataLayout(), PtrValue, Alignment,
+  OffsetValue);
+  }
+
   //======//
   // Statement Emission
   //======//
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -168,6 +168,16 @@
 Aligned->getSpellingListIndex());
 }
 
+static void instantiateDependentAllocAlignAttr(
+Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
+const AllocAlignAttr *Align, Decl *New) {
+  Expr *Param = IntegerLiteral::Create(
+  S.getASTContext(), llvm::APInt(64, Align->getParamIndex()),
+  S.getASTContext().UnsignedLongLongTy, Align->getLocation());
+  S.AddAllocAlignAttr(Align->getLocation(), New, Param,
+  Align->getSpellingListIndex());
+}
+
 static Expr *instantiateDependentFunctionAttrCondition(
 Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
 const Attr *A, Expr *OldCond, const Decl *Tmpl, FunctionDecl *New) {
@@ -380,6 +390,12 @@
   continue;
 }
 
+if (const auto *AllocAlign = dyn_cast(TmplAttr)) {
+  instantiateDependentAllocAlignAttr(*this, TemplateArgs, AllocAlign, New);
+  continue;
+}
+
+
 if (const auto *EnableIf = dyn_cast(TmplAttr)) {
   instantiateDependentEnableIfAttr(*this, TemplateArgs, EnableIf, Tmpl,
cast(New));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -218,21 +218,45 @@
std::greater());
 }
 
+/// \brief A helper function to provide Attribute Location for the Attr types
+/// AND the AttributeList.
+template 
+static typename std::enable_if::value,
+   SourceLocation>::type
+getAttrLoc(const AttrInfo &Attr) {
+  return Attr.getLocation();
+}
+static SourceLocation getAttrLoc(const clang::AttributeList &Attr) {
+  return Attr.getLoc();
+}
+
+/// \brief A helper function to provide Attribute Name for the Attr types
+/// AND the AttributeList.
+template 
+static typename std::enable_if::value,
+   const AttrInfo *>::type
+getAttrName(const AttrInfo &Attr) {
+  return &Attr;
+}
+const IdentifierInfo *getAttrName(const clang::AttributeList &Attr) {
+  return Attr.getName();
+}
+
 /// \brief If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
-static bool checkUInt32Argument(Sema &S, const AttributeList &Attr,
-const Expr *Expr, uint32_t &Val,
-unsigned Idx = UINT_MAX) {
+template
+static bool checkUInt32Argument(Sema &S, const AttrInfo& Attr, const Expr *Expr,
+uint32_t &Val, unsigned Idx = UINT_MAX) {
   llvm::APSInt I(32);
   if (Expr->isTypeDependent() || Expr->isValueDependent() ||
   !Expr->isIntegerConstantExpr(I, S.Context)) {
 if (Idx != UINT_MAX)
-  S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-<< Attr.getName() << Idx << AANT_ArgumentIntegerConstant
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_n_type)
+

r299033 - Use FPContractModeKind universally

[Adam Nemet via cfe-commits]

Author: anemet
Date: Wed Mar 29 16:54:24 2017
New Revision: 299033

URL: http://llvm.org/viewvc/llvm-project?rev=299033&view=rev
Log:
Use FPContractModeKind universally

FPContractModeKind is the codegen option flag which is already ternary (off,
on, fast).  This makes it universally the type for the contractable info
across the front-end:

* In FPOptions (i.e. in the Sema + in the expression nodes).
* In LangOpts::DefaultFPContractMode which is the option that initializes
FPOptions in the Sema.

Another way to look at this change is that before fp-contractable on/off were
the only states handled to the front-end:
 * For "on", FMA folding was performed by  the front-end
 * For "fast", we simply forwarded the flag to TargetOptions to handle it in
 LLVM

Now off/on/fast are all exposed because for fast we will generate
fast-math-flags during CodeGen.

This is toward moving fp-contraction=fast from an LLVM TargetOption to a
FastMathFlag in order to fix PR25721.

---
This is a recommit of r299027 with an adjustment to the test
CodeGenCUDA/fp-contract.cu.  The test assumed that even
though -ffp-contract=on is passed FE-based folding of FMA won't happen.

This is obviously wrong since the user is asking for this explicitly with the
option.  CUDA is different that -ffp-contract=fast is on by default.

The test used to "work" because contract=fast and contract=on were maintained
separately and we didn't fold in the FE because contract=fast was on due to
the target-default.  This patch consolidates the contract=on/fast/off state
into a ternary state hence the change in behavior.
---

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/test/CodeGenCUDA/fp-contract.cu

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=299033&r1=299032&r2=299033&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Wed Mar 29 16:54:24 2017
@@ -2920,7 +2920,7 @@ private:
 
   // This is only meaningful for operations on floating point types and 0
   // otherwise.
-  unsigned FPFeatures : 1;
+  unsigned FPFeatures : 2;
   SourceLocation OpLoc;
 
   enum { LHS, RHS, END_EXPR };
@@ -3078,8 +3078,8 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractable() const {
-return FPOptions(FPFeatures).isFPContractable();
+  bool isFPContractableWithinStatement() const {
+return FPOptions(FPFeatures).allowFPContractWithinStatement();
   }
 
 protected:

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=299033&r1=299032&r2=299033&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Wed Mar 29 16:54:24 2017
@@ -117,7 +117,9 @@ public:
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractable() const { return FPFeatures.isFPContractable(); }
+  bool isFPContractableWithinStatement() const {
+return FPFeatures.allowFPContractWithinStatement();
+  }
 
   friend class ASTStmtReader;
   friend class ASTStmtWriter;

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=299033&r1=299032&r2=299033&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Mar 29 16:54:24 2017
@@ -216,7 +216,8 @@ BENIGN_LANGOPT(DebuggerObjCLiteral , 1,
 BENIGN_LANGOPT(SpellChecking , 1, 1, "spell-checking")
 LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating 
point constants as single precision constants")
 LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math")
-LANGOPT(DefaultFPContract , 1, 0, "FP_CONTRACT")
+/// \brief FP_CONTRACT mode (on/off/fast).
+ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP 
contraction type")
 LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
 LANGOPT(HexagonQdsp6Compat , 1, 0, "hexagon-qdsp6 backward compatibility")
 LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C 

[PATCH] D29599: Clang Changes for alloc_align

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I see that GCC is up to its same parameter-indexing shenanigans again.




Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter. The
+alignment parameter is one-based. In the case of member functions, the

"that the return value of the function ... is at least as aligned as the
value of the indicated parameter.  The parameter is given by its index
in the list of formal parameters; the first parameter has index 1 unless
the function is a C++ non-static member function, in which case the
first parameter has index 2 to account for the implicit ``this`` parameter."

What's the behavior of this attribute if the given attribute is not a power of 
2?
Does it silently have no effect, or is it U.B.?



Comment at: include/clang/Basic/AttrDocs.td:265
+  // The returned pointer has the alignment specified by the second visible
+  // parameter, however it must be adjusted for the implicit 'this' parameter.
+  void *Foo::b(void *v, size_t align) __attribute__((alloc_align(3)));

"The returned pointer has the alignment specified by the second formal
parameter, 'align'.  However, for the purposes of the alloc_align attribute,
the parameter index is 3 because of the implicit 'this' parameter."



Comment at: include/clang/Basic/AttrDocs.td:270
+condition that the code already ensures is true. It does not cause the compiler
+to enforce the provided alignment assumption.
+  }];

"Note that this attribute merely informs the compiler that a function always
returns a sufficiently aligned pointer.  It does not cause the compiler to emit
code to enforce that alignment.  The behavior is undefined if the returned
pointer is not sufficiently aligned."



Comment at: lib/CodeGen/CGCall.cpp:4352
+} else if (const auto *AA = TargetDecl->getAttr()) {
+  llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+  EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);

IRCallArgs is not one-to-one with the list of formal parameters.  You need to 
be taking the value out of CallArgList.  Three test cases come to mind:

1. There's an earlier struct argument which expands to no IR arguments.
2. There's an earlier struct argument which expands to multiple IR arguments.
3. The alignment argument itself expands to multiple IR arguments.  For 
example, __int128_t on x86-64.


https://reviews.llvm.org/D29599



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

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/CodeGen/CGCoroutine.cpp:225
+  void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+  }

rnk wrote:
> This will be called twice: once for a normal exit and once for exceptional 
> exit. In general, double emitting a `Stmt*` is not safe, since it might 
> contain a VarDecl or a LabelDecl, but this usage is safe because 
> `SubStmtBuilder::makeNewAndDeleteExpr()` builds two calls that can't declare 
> anything. That is *definitely* worth a comment. :)
Thank you very much for the review!
Would switching the type of Deallocate from Stmt* to Expr* address this 
concern? 


https://reviews.llvm.org/D31460



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

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter. The
+alignment parameter is one-based. In the case of member functions, the

rjmccall wrote:
> "that the return value of the function ... is at least as aligned as the
> value of the indicated parameter.  The parameter is given by its index
> in the list of formal parameters; the first parameter has index 1 unless
> the function is a C++ non-static member function, in which case the
> first parameter has index 2 to account for the implicit ``this`` parameter."
> 
> What's the behavior of this attribute if the given attribute is not a power 
> of 2?
> Does it silently have no effect, or is it U.B.?
Thanks for the wording, I like this.

It isn't checked for that, so I'll say UB.  A little bit of a caveat emptor 
here.



Comment at: include/clang/Basic/AttrDocs.td:270
+condition that the code already ensures is true. It does not cause the compiler
+to enforce the provided alignment assumption.
+  }];

rjmccall wrote:
> "Note that this attribute merely informs the compiler that a function always
> returns a sufficiently aligned pointer.  It does not cause the compiler to 
> emit
> code to enforce that alignment.  The behavior is undefined if the returned
> pointer is not sufficiently aligned."
Thanks again!



Comment at: lib/CodeGen/CGCall.cpp:4352
+} else if (const auto *AA = TargetDecl->getAttr()) {
+  llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+  EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);

rjmccall wrote:
> IRCallArgs is not one-to-one with the list of formal parameters.  You need to 
> be taking the value out of CallArgList.  Three test cases come to mind:
> 
> 1. There's an earlier struct argument which expands to no IR arguments.
> 2. There's an earlier struct argument which expands to multiple IR arguments.
> 3. The alignment argument itself expands to multiple IR arguments.  For 
> example, __int128_t on x86-64.
Awesome, thanks for the info!  I'll change that.


https://reviews.llvm.org/D29599



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

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/CodeGen/CGCoroutine.cpp:225
+  void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+  }

GorNishanov wrote:
> rnk wrote:
> > This will be called twice: once for a normal exit and once for exceptional 
> > exit. In general, double emitting a `Stmt*` is not safe, since it might 
> > contain a VarDecl or a LabelDecl, but this usage is safe because 
> > `SubStmtBuilder::makeNewAndDeleteExpr()` builds two calls that can't 
> > declare anything. That is *definitely* worth a comment. :)
> Thank you very much for the review!
> Would switching the type of Deallocate from Stmt* to Expr* address this 
> concern? 
Not really, because gnu statement exprs mean Exprs can contain Decls as well. =(


https://reviews.llvm.org/D31460



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

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl created this revision.
Herald added subscribers: t-tye, tpr, dstuttard, yaxunl, wdng.

https://reviews.llvm.org/D31482

Files:
  llvm/tools/clang/lib/Basic/Targets.cpp
  llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl


Index: llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
===
--- llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
+++ llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
@@ -0,0 +1,13 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - %s | FileCheck --check-prefix=DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -target-feature +fp32-denormals %s | FileCheck 
--check-prefix=FEATURE_FP32_DENORMALS_ON %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -target-feature -fp32-denormals %s | FileCheck 
--check-prefix=FEATURE_FP32_DENORMALS_OFF %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -cl-denorms-are-zero %s | FileCheck 
--check-prefix=OPT_DENORMS_ARE_ZERO %s
+
+// DEFAULT: +fp32-denormals
+// FEATURE_FP32_DENORMALS_ON: +fp32-denormals
+// FEATURE_FP32_DENORMALS_OFF: -fp32-denormals
+// OPT_DENORMS_ARE_ZERO: -fp32-denormals
+
+kernel void gfx9_fp32_denorms() {}
Index: llvm/tools/clang/lib/Basic/Targets.cpp
===
--- llvm/tools/clang/lib/Basic/Targets.cpp
+++ llvm/tools/clang/lib/Basic/Targets.cpp
@@ -2109,9 +2109,12 @@
   bool hasFP64:1;
   bool hasFMAF:1;
   bool hasLDEXPF:1;
-  bool hasFullSpeedFP32Denorms:1;
   const AddrSpace AS;
 
+  static bool hasFullSpeedFP32Denorms(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }
+
   static bool isAMDGCN(const llvm::Triple &TT) {
 return TT.getArch() == llvm::Triple::amdgcn;
   }
@@ -2127,7 +2130,6 @@
   hasFP64(false),
   hasFMAF(false),
   hasLDEXPF(false),
-  hasFullSpeedFP32Denorms(false),
   AS(isGenericZero(Triple)){
 if (getTriple().getArch() == llvm::Triple::amdgcn) {
   hasFP64 = true;
@@ -2196,7 +2198,8 @@
 hasFP64Denormals = true;
 }
 if (!hasFP32Denormals)
-  TargetOpts.Features.push_back((Twine(hasFullSpeedFP32Denorms &&
+  TargetOpts.Features.push_back(
+  (Twine(hasFullSpeedFP32Denorms(TargetOpts.CPU) &&
   !CGOpts.FlushDenorm ? '+' : '-') + Twine("fp32-denormals")).str());
 // Always do not flush fp64 or fp16 denorms.
 if (!hasFP64Denormals && hasFP64)


Index: llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
===
--- llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
+++ llvm/tools/clang/test/CodeGenOpenCL/gfx9-fp32-denorms.cl
@@ -0,0 +1,13 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - %s | FileCheck --check-prefix=DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -target-feature +fp32-denormals %s | FileCheck --check-prefix=FEATURE_FP32_DENORMALS_ON %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -target-feature -fp32-denormals %s | FileCheck --check-prefix=FEATURE_FP32_DENORMALS_OFF %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -cl-denorms-are-zero %s | FileCheck --check-prefix=OPT_DENORMS_ARE_ZERO %s
+
+// DEFAULT: +fp32-denormals
+// FEATURE_FP32_DENORMALS_ON: +fp32-denormals
+// FEATURE_FP32_DENORMALS_OFF: -fp32-denormals
+// OPT_DENORMS_ARE_ZERO: -fp32-denormals
+
+kernel void gfx9_fp32_denorms() {}
Index: llvm/tools/clang/lib/Basic/Targets.cpp
===
--- llvm/tools/clang/lib/Basic/Targets.cpp
+++ llvm/tools/clang/lib/Basic/Targets.cpp
@@ -2109,9 +2109,12 @@
   bool hasFP64:1;
   bool hasFMAF:1;
   bool hasLDEXPF:1;
-  bool hasFullSpeedFP32Denorms:1;
   const AddrSpace AS;
 
+  static bool hasFullSpeedFP32Denorms(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }
+
   static bool isAMDGCN(const llvm::Triple &TT) {
 return TT.getArch() == llvm::Triple::amdgcn;
   }
@@ -2127,7 +2130,6 @@
   hasFP64(false),
   hasFMAF(false),
   hasLDEXPF(false),
-  hasFullSpeedFP32Denorms(false),
   AS(isGenericZero(Triple)){
 if (getTriple().getArch() == llvm::Triple::amdgcn) {
   hasFP64 = true;
@@ -2196,7 +2198,8 @@
 hasFP64Denormals = true;
 }
 if (!hasFP32Denormals)
-  TargetOpts.Features.push_back((Twine(hasFullSpeedFP32Denorms &&
+  TargetOpts.Features.push_back(
+  (Twine(hasFullSpeedFP32Denorms(TargetOpts.CPU) &&
   !CGOpts.FlushDenorm ? '+' : '-') + Twine("fp32-denormals")).str());
 // Always do

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/tools/clang/lib/Basic/Targets.cpp:2114-2116
+  static bool hasFullSpeedFP32Denorms(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }

This is misleading since it was true on VI as well. I think just FMA rate 
changed


https://reviews.llvm.org/D31482



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

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: llvm/tools/clang/lib/Basic/Targets.cpp:2114-2116
+  static bool hasFullSpeedFP32Denorms(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }

arsenm wrote:
> This is misleading since it was true on VI as well. I think just FMA rate 
> changed
Yes, GFX8 supports f32 denorms at full speed too.  However, it doesn't have a 
full speed fma, so we didh't enable it then since it caused too many mad-heavy 
apps to slow down.


https://reviews.llvm.org/D31482



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

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/tools/clang/lib/Basic/Targets.cpp:2114-2116
+  static bool hasFullSpeedFP32Denorms(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }

kzhuravl wrote:
> arsenm wrote:
> > This is misleading since it was true on VI as well. I think just FMA rate 
> > changed
> Yes, GFX8 supports f32 denorms at full speed too.  However, it doesn't have a 
> full speed fma, so we didh't enable it then since it caused too many 
> mad-heavy apps to slow down.
Yes, so the name should refer to FMA rather than just fp32 denorms


https://reviews.llvm.org/D31482



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

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/CodeGen/CGCoroutine.cpp:225
+  void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+  }

rnk wrote:
> GorNishanov wrote:
> > rnk wrote:
> > > This will be called twice: once for a normal exit and once for 
> > > exceptional exit. In general, double emitting a `Stmt*` is not safe, 
> > > since it might contain a VarDecl or a LabelDecl, but this usage is safe 
> > > because `SubStmtBuilder::makeNewAndDeleteExpr()` builds two calls that 
> > > can't declare anything. That is *definitely* worth a comment. :)
> > Thank you very much for the review!
> > Would switching the type of Deallocate from Stmt* to Expr* address this 
> > concern? 
> Not really, because gnu statement exprs mean Exprs can contain Decls as well. 
> =(
Ahh. Okay, comment it is.


https://reviews.llvm.org/D31460



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

[PATCH] D29599: Clang Changes for alloc_align

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 93421.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

Fixes based on John's comments.  A little bit of extra work was required to get 
the correct Value from the attribute, but impact was minimal.


https://reviews.llvm.org/D29599

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/CodeGen/alloc-align-attr.c
  test/Sema/alloc-align-attr.c
  test/SemaCXX/alloc-align-attr.cpp

Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3703,6 +3703,15 @@
 
   llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
 
+  // Support for getting the Value for the parameter identified
+  // by the AllocAlignAttr.
+  const Decl *TargetDecl = Callee.getAbstractInfo().getCalleeDecl();
+  llvm::Value *AllocAlignParam = nullptr;
+  unsigned AllocAlignParamIndex = (std::numeric_limits::max)();
+  if (TargetDecl)
+if (const auto *AA = TargetDecl->getAttr())
+  AllocAlignParamIndex = AA->getParamIndex() - 1;
+
   // 1. Set up the arguments.
 
   // If we're using inalloca, insert the allocation after the stack save.
@@ -4009,6 +4018,9 @@
   assert(IRArgPos == FirstIRArg + NumIRArgs);
   break;
 }
+
+if (ArgNo == AllocAlignParamIndex)
+  AllocAlignParam = IRCallArgs[FirstIRArg];
   }
 
   llvm::Value *CalleePtr = Callee.getFunctionPointer();
@@ -4337,7 +4349,6 @@
   } ();
 
   // Emit the assume_aligned check on the return value.
-  const Decl *TargetDecl = Callee.getAbstractInfo().getCalleeDecl();
   if (Ret.isScalar() && TargetDecl) {
 if (const auto *AA = TargetDecl->getAttr()) {
   llvm::Value *OffsetValue = nullptr;
@@ -4348,7 +4359,8 @@
   llvm::ConstantInt *AlignmentCI = cast(Alignment);
   EmitAlignmentAssumption(Ret.getScalarVal(), AlignmentCI->getZExtValue(),
   OffsetValue);
-}
+} else if (AllocAlignParam && TargetDecl->hasAttr())
+  EmitAlignmentAssumption(Ret.getScalarVal(), AllocAlignParam);
   }
 
   return Ret;
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2465,6 +2465,12 @@
   PeepholeProtection protectFromPeepholes(RValue rvalue);
   void unprotectFromPeepholes(PeepholeProtection protection);
 
+  void EmitAlignmentAssumption(llvm::Value *PtrValue, llvm::Value *Alignment,
+   llvm::Value *OffsetValue = nullptr) {
+Builder.CreateAlignmentAssumption(CGM.getDataLayout(), PtrValue, Alignment,
+  OffsetValue);
+  }
+
   //======//
   // Statement Emission
   //======//
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -168,6 +168,16 @@
 Aligned->getSpellingListIndex());
 }
 
+static void instantiateDependentAllocAlignAttr(
+Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
+const AllocAlignAttr *Align, Decl *New) {
+  Expr *Param = IntegerLiteral::Create(
+  S.getASTContext(), llvm::APInt(64, Align->getParamIndex()),
+  S.getASTContext().UnsignedLongLongTy, Align->getLocation());
+  S.AddAllocAlignAttr(Align->getLocation(), New, Param,
+  Align->getSpellingListIndex());
+}
+
 static Expr *instantiateDependentFunctionAttrCondition(
 Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
 const Attr *A, Expr *OldCond, const Decl *Tmpl, FunctionDecl *New) {
@@ -380,6 +390,12 @@
   continue;
 }
 
+if (const auto *AllocAlign = dyn_cast(TmplAttr)) {
+  instantiateDependentAllocAlignAttr(*this, TemplateArgs, AllocAlign, New);
+  continue;
+}
+
+
 if (const auto *EnableIf = dyn_cast(TmplAttr)) {
   instantiateDependentEnableIfAttr(*this, TemplateArgs, EnableIf, Tmpl,
cast(New));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -218,21 +218,45 @@
std::greater());
 }
 
+/// \brief A helper function to provide Attribute Location for the Attr types
+/// AND the AttributeList.
+template 
+static typename std::enable_if::value,
+   SourceLocation>::type
+getAttrLoc(const AttrInfo &Attr) {
+  return Attr.getLocation();
+}
+static SourceLocation getAttrLoc(con

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl updated this revision to Diff 93422.
kzhuravl marked an inline comment as done.
kzhuravl added a comment.

Address review feedback.


https://reviews.llvm.org/D31482

Files:
  lib/Basic/Targets.cpp
  test/CodeGenOpenCL/gfx9-fp32-denorms.cl


Index: test/CodeGenOpenCL/gfx9-fp32-denorms.cl
===
--- test/CodeGenOpenCL/gfx9-fp32-denorms.cl
+++ test/CodeGenOpenCL/gfx9-fp32-denorms.cl
@@ -0,0 +1,13 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - %s | FileCheck --check-prefix=DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -target-feature +fp32-denormals %s | FileCheck 
--check-prefix=FEATURE_FP32_DENORMALS_ON %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -target-feature -fp32-denormals %s | FileCheck 
--check-prefix=FEATURE_FP32_DENORMALS_OFF %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S 
-emit-llvm -o - -cl-denorms-are-zero %s | FileCheck 
--check-prefix=OPT_DENORMS_ARE_ZERO %s
+
+// DEFAULT: +fp32-denormals
+// FEATURE_FP32_DENORMALS_ON: +fp32-denormals
+// FEATURE_FP32_DENORMALS_OFF: -fp32-denormals
+// OPT_DENORMS_ARE_ZERO: -fp32-denormals
+
+kernel void gfx9_fp32_denorms() {}
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2109,9 +2109,12 @@
   bool hasFP64:1;
   bool hasFMAF:1;
   bool hasLDEXPF:1;
-  bool hasFullSpeedFP32Denorms:1;
   const AddrSpace AS;
 
+  static bool hasFullSpeedFMA(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }
+
   static bool isAMDGCN(const llvm::Triple &TT) {
 return TT.getArch() == llvm::Triple::amdgcn;
   }
@@ -2127,7 +2130,6 @@
   hasFP64(false),
   hasFMAF(false),
   hasLDEXPF(false),
-  hasFullSpeedFP32Denorms(false),
   AS(isGenericZero(Triple)){
 if (getTriple().getArch() == llvm::Triple::amdgcn) {
   hasFP64 = true;
@@ -2196,7 +2198,8 @@
 hasFP64Denormals = true;
 }
 if (!hasFP32Denormals)
-  TargetOpts.Features.push_back((Twine(hasFullSpeedFP32Denorms &&
+  TargetOpts.Features.push_back(
+  (Twine(hasFullSpeedFMA(TargetOpts.CPU) &&
   !CGOpts.FlushDenorm ? '+' : '-') + Twine("fp32-denormals")).str());
 // Always do not flush fp64 or fp16 denorms.
 if (!hasFP64Denormals && hasFP64)


Index: test/CodeGenOpenCL/gfx9-fp32-denorms.cl
===
--- test/CodeGenOpenCL/gfx9-fp32-denorms.cl
+++ test/CodeGenOpenCL/gfx9-fp32-denorms.cl
@@ -0,0 +1,13 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - %s | FileCheck --check-prefix=DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -target-feature +fp32-denormals %s | FileCheck --check-prefix=FEATURE_FP32_DENORMALS_ON %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -target-feature -fp32-denormals %s | FileCheck --check-prefix=FEATURE_FP32_DENORMALS_OFF %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -emit-llvm -o - -cl-denorms-are-zero %s | FileCheck --check-prefix=OPT_DENORMS_ARE_ZERO %s
+
+// DEFAULT: +fp32-denormals
+// FEATURE_FP32_DENORMALS_ON: +fp32-denormals
+// FEATURE_FP32_DENORMALS_OFF: -fp32-denormals
+// OPT_DENORMS_ARE_ZERO: -fp32-denormals
+
+kernel void gfx9_fp32_denorms() {}
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2109,9 +2109,12 @@
   bool hasFP64:1;
   bool hasFMAF:1;
   bool hasLDEXPF:1;
-  bool hasFullSpeedFP32Denorms:1;
   const AddrSpace AS;
 
+  static bool hasFullSpeedFMA(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }
+
   static bool isAMDGCN(const llvm::Triple &TT) {
 return TT.getArch() == llvm::Triple::amdgcn;
   }
@@ -2127,7 +2130,6 @@
   hasFP64(false),
   hasFMAF(false),
   hasLDEXPF(false),
-  hasFullSpeedFP32Denorms(false),
   AS(isGenericZero(Triple)){
 if (getTriple().getArch() == llvm::Triple::amdgcn) {
   hasFP64 = true;
@@ -2196,7 +2198,8 @@
 hasFP64Denormals = true;
 }
 if (!hasFP32Denormals)
-  TargetOpts.Features.push_back((Twine(hasFullSpeedFP32Denorms &&
+  TargetOpts.Features.push_back(
+  (Twine(hasFullSpeedFMA(TargetOpts.CPU) &&
   !CGOpts.FlushDenorm ? '+' : '-') + Twine("fp32-denormals")).str());
 // Always do not flush fp64 or fp16 denorms.
 if (!hasFP64Denormals && hasFP64)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

r299037 - Add the -grecord-gcc-switches option and pass the flags down on the compile unit.

[Eric Christopher via cfe-commits]

Author: echristo
Date: Wed Mar 29 18:34:20 2017
New Revision: 299037

URL: http://llvm.org/viewvc/llvm-project?rev=299037&view=rev
Log:
Add the -grecord-gcc-switches option and pass the flags down on the compile 
unit.

Patch by Zhizhou Yang

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/debug-options.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=299037&r1=299036&r2=299037&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Mar 29 18:34:20 2017
@@ -2728,7 +2728,8 @@ void Clang::ConstructJob(Compilation &C,
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@ void Clang::ConstructJob(Compilation &C,
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto &Arg : Args)
   Arg->render(Args, OriginalArgs);

Modified: cfe/trunk/test/Driver/debug-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/debug-options.c?rev=299037&r1=299036&r2=299037&view=diff
==
--- cfe/trunk/test/Driver/debug-options.c (original)
+++ cfe/trunk/test/Driver/debug-options.c Wed Mar 29 18:34:20 2017
@@ -117,8 +117,18 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +204,17 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -grecord-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
+// GRECORD_OPT: -### -c -O3 -ffunction-sections -grecord-gcc-switches
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections


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

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

Committed thusly:
echristo@athyra ~/s/l/t/clang> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
M   lib/Driver/ToolChains/Clang.cpp
M   test/Driver/debug-options.c
Committed r299037


https://reviews.llvm.org/D30760



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

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: include/clang/Driver/XRayArgs.h:22
+class XRayArgs {
+  std::vector AlwaysInstrumenFiles;
+  std::vector NeverInstrumentFiles;

Any reason to omit the 't' in "InstrumentFiles"?


https://reviews.llvm.org/D30388



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

[PATCH] D31167: Use FPContractModeKind universally

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217
   /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this.
-  unsigned fp_contract : 1;
+  LangOptions::FPContractModeKind fp_contract : 2;
 };

Please do not use bitfields with enum types, it's a good way to break the build 
on Windows. This change triggered this clang-cl warning:
```
C:\src\llvm-project\clang\include\clang/Basic/LangOptions.h(208,17):  warning: 
implicit truncation from 'clang::LangOptions::FPContractModeKind' to bit-field 
changes value from 2 to -2 [-Wbitfield-constant-conversion]
fp_contract = LangOptions::FPC_Fast;
^ ~
```


Repository:
  rL LLVM

https://reviews.llvm.org/D31167



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

r299041 - [XRay] Add -fxray-{always, never}-instrument= flags to clang

[Dean Michael Berris via cfe-commits]

Author: dberris
Date: Wed Mar 29 19:29:36 2017
New Revision: 299041

URL: http://llvm.org/viewvc/llvm-project?rev=299041&view=rev
Log:
[XRay] Add -fxray-{always,never}-instrument= flags to clang

Summary:
The -fxray-always-instrument= and -fxray-never-instrument= flags take
filenames that are used to imbue the XRay instrumentation attributes
using a whitelist mechanism (similar to the sanitizer special cases
list). We use the same syntax and semantics as the sanitizer blacklists
files in the implementation.

As implemented, we respect the attributes that are already defined in
the source file (i.e. those that have the
[[clang::xray_{always,never}_instrument]] attributes) before applying
the always/never instrument lists.

Reviewers: rsmith, chandlerc

Subscribers: jfb, mgorny, cfe-commits

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

Added:
cfe/trunk/include/clang/Basic/XRayLists.h
cfe/trunk/include/clang/Driver/XRayArgs.h
cfe/trunk/lib/Basic/XRayLists.cpp
cfe/trunk/lib/Driver/XRayArgs.cpp
cfe/trunk/test/CodeGen/xray-always-instrument.cpp
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Basic/CMakeLists.txt
cfe/trunk/lib/Basic/LangOptions.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/Driver/CMakeLists.txt
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=299041&r1=299040&r2=299041&view=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Mar 29 19:29:36 2017
@@ -39,6 +39,7 @@
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/XRayLists.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
@@ -475,6 +476,10 @@ private:
   /// entities should not be instrumented.
   std::unique_ptr SanitizerBL;
 
+  /// \breif Function filtering mehcanism to determine whether a given function
+  /// should be imbued with the XRay "always" or "never" attributes.
+  std::unique_ptr XRayFilter;
+
   /// \brief The allocator used to create AST objects.
   ///
   /// AST objects are never destructed; rather, all memory associated with the
@@ -657,6 +662,10 @@ public:
 return *SanitizerBL;
   }
 
+  const XRayFunctionFilter &getXRayFilter() const {
+return *XRayFilter;
+  }
+
   DiagnosticsEngine &getDiagnostics() const;
 
   FullSourceLoc getFullLoc(SourceLocation Loc) const {

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=299041&r1=299040&r2=299041&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Mar 29 19:29:36 2017
@@ -263,6 +263,8 @@ LANGOPT(SanitizeAddressFieldPadding, 2,
"field padding (0: none, 1:least "
"aggressive, 2: more aggressive)")
 
+LANGOPT(XRayInstrument, 1, 0, "controls whether to do XRay instrumentation")
+
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=299041&r1=299040&r2=299041&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Wed Mar 29 19:29:36 2017
@@ -102,6 +102,16 @@ public:
   /// (files, functions, variables) should not be instrumented.
   std::vector SanitizerBlacklistFiles;
 
+  /// \brief Paths to the XRay "always instrument" files specifying which
+  /// objects (files, functions, variables) should be imbued with the XRay
+  /// "always instrument" attribute.
+  std::vector XRayAlwaysInstrumentFiles;
+
+  /// \brief Paths to the XRay "never instrument" files specifying which
+  /// objects (files, functions, variables) should be imbued with the XRay
+  /// "never instrument" attribute.
+  std::vector XRayNeverInstrumentFiles;
+
   clang::ObjCRuntime ObjCRuntime;
 
   std::string ObjCConstantStringClass;

Added: cfe/trunk/include/clang/Basic/XRayLists.h
URL: 
http://llvm.or

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
dberris marked an inline comment as done.
Closed by commit rL299041: [XRay] Add -fxray-{always,never}-instrument= flags 
to clang (authored by dberris).

Changed prior to commit:
  https://reviews.llvm.org/D30388?vs=92766&id=93428#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30388

Files:
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Basic/LangOptions.h
  cfe/trunk/include/clang/Basic/XRayLists.h
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/ToolChain.h
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/Basic/CMakeLists.txt
  cfe/trunk/lib/Basic/LangOptions.cpp
  cfe/trunk/lib/Basic/XRayLists.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/lib/Driver/CMakeLists.txt
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/xray-always-instrument.cpp

Index: cfe/trunk/lib/Basic/XRayLists.cpp
===
--- cfe/trunk/lib/Basic/XRayLists.cpp
+++ cfe/trunk/lib/Basic/XRayLists.cpp
@@ -0,0 +1,53 @@
+//===--- XRayFunctionFilter.cpp - XRay automatic-attribution --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// User-provided filters for always/never XRay instrumenting certain functions.
+//
+//===--===//
+#include "clang/Basic/XRayLists.h"
+
+using namespace clang;
+
+XRayFunctionFilter::XRayFunctionFilter(
+ArrayRef AlwaysInstrumentPaths,
+ArrayRef NeverInstrumentPaths, SourceManager &SM)
+: AlwaysInstrument(
+  llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)),
+  NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)),
+  SM(SM) {}
+
+XRayFunctionFilter::ImbueAttribute
+XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const {
+  // First apply the always instrument list, than if it isn't an "always" see
+  // whether it's treated as a "never" instrument function.
+  if (AlwaysInstrument->inSection("fun", FunctionName))
+return ImbueAttribute::ALWAYS;
+  if (NeverInstrument->inSection("fun", FunctionName))
+return ImbueAttribute::NEVER;
+  return ImbueAttribute::NONE;
+}
+
+XRayFunctionFilter::ImbueAttribute
+XRayFunctionFilter::shouldImbueFunctionsInFile(StringRef Filename,
+   StringRef Category) const {
+  if (AlwaysInstrument->inSection("src", Filename, Category))
+return ImbueAttribute::ALWAYS;
+  if (NeverInstrument->inSection("src", Filename, Category))
+return ImbueAttribute::NEVER;
+  return ImbueAttribute::NONE;
+}
+
+XRayFunctionFilter::ImbueAttribute
+XRayFunctionFilter::shouldImbueLocation(SourceLocation Loc,
+StringRef Category) const {
+  if (!Loc.isValid())
+return ImbueAttribute::NONE;
+  return this->shouldImbueFunctionsInFile(SM.getFilename(SM.getFileLoc(Loc)),
+  Category);
+}
Index: cfe/trunk/lib/Basic/LangOptions.cpp
===
--- cfe/trunk/lib/Basic/LangOptions.cpp
+++ cfe/trunk/lib/Basic/LangOptions.cpp
@@ -33,6 +33,8 @@
   // sanitizer options (this affects __has_feature(address_sanitizer) etc).
   Sanitize.clear();
   SanitizerBlacklistFiles.clear();
+  XRayAlwaysInstrumentFiles.clear();
+  XRayNeverInstrumentFiles.clear();
 
   CurrentModule.clear();
   IsHeaderFile = false;
Index: cfe/trunk/lib/Basic/CMakeLists.txt
===
--- cfe/trunk/lib/Basic/CMakeLists.txt
+++ cfe/trunk/lib/Basic/CMakeLists.txt
@@ -90,6 +90,7 @@
   VersionTuple.cpp
   VirtualFileSystem.cpp
   Warnings.cpp
+  XRayLists.cpp
   ${version_inc}
   )
 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -2307,6 +2307,14 @@
   Opts.SanitizeAddressFieldPadding =
   getLastArgIntValue(Args, OPT_fsanitize_address_field_padding, 0, Diags);
   Opts.SanitizerBlacklistFiles = Args.getAllArgValues(OPT_fsanitize_blacklist);
+
+  // -fxray-{always,never}-instrument= filenames.
+  Opts.XRayInstrument =
+  Args.hasFlag(OPT_fxray_instrument, OPT_fnoxray_instrument, false);
+  Opts.XRayAlwaysInstrumentFiles =
+  Args.get

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: include/clang/Driver/XRayArgs.h:22
+class XRayArgs {
+  std::vector AlwaysInstrumenFiles;
+  std::vector NeverInstrumentFiles;

rnk wrote:
> Any reason to omit the 't' in "InstrumentFiles"?
Wow, good catch -- no good reason, except for lack of a good spellcheck!


Repository:
  rL LLVM

https://reviews.llvm.org/D30388



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

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Matt Arsenault via Phabricator via cfe-commits]

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

LGTM with f32 clarification




Comment at: lib/Basic/Targets.cpp:2114
 
+  static bool hasFullSpeedFMA(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;

FMAF32?



Comment at: lib/Basic/Targets.cpp:2115
+  static bool hasFullSpeedFMA(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;
+  }

We should probably add a new subtarget feature for this, but that's a separate 
patch


https://reviews.llvm.org/D31482



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

[PATCH] D31482: [AMDGPU][GFX9] Set +fp32-denormals for >=gfx900 unless -cl-denorms-are-zero is set

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: lib/Basic/Targets.cpp:2114
 
+  static bool hasFullSpeedFMA(StringRef GPUName) {
+return parseAMDGCNName(GPUName) >= GK_GFX9;

arsenm wrote:
> FMAF32?
Actually this also needs to specify full speed FMA with denorms. Full rate FMA 
is already a subtarget feature when denorms are disabled


https://reviews.llvm.org/D31482



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

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

[Andrew Ford via Phabricator via cfe-commits]

andrewford added a comment.

Hi, this breaks the android build due to use of std::to_string.  
llvm::to_string should be used instead.  I would fix it myself, but don't have 
commit access.  Thanks in advance!


Repository:
  rL LLVM

https://reviews.llvm.org/D30388



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

[PATCH] D31486: libc++ testing: allow to provide a path for `use_system_cxx_lib`

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini created this revision.

As we're trying to setup testing / bots for all shipping version of libc++
on macOS/iOS, we'll need to be able to pass a path to where to find the
dylib for each previous version of the OS.


https://reviews.llvm.org/D31486

Files:
  utils/libcxx/test/config.py


Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -275,7 +275,13 @@
 # compatibility between the current headers and a shipping dynamic
 # library.
 # Default to testing against the locally built libc++ library.
-self.use_system_cxx_lib = self.get_lit_bool('use_system_cxx_lib', 
False)
+self.use_system_cxx_lib = self.get_lit_conf('use_system_cxx_lib')
+if self.use_system_cxx_lib == 'true':
+self.use_system_cxx_lib = True
+elif self.use_system_cxx_lib == 'false':
+self.use_system_cxx_lib = False
+else:
+assert os.path.isdir(self.use_system_cxx_lib)
 self.lit_config.note(
 "inferred use_system_cxx_lib as: %r" % self.use_system_cxx_lib)
 
@@ -719,6 +725,13 @@
 self.cxx_runtime_root]
 elif self.is_windows and self.link_shared:
 self.add_path(self.exec_env, self.cxx_runtime_root)
+elif os.path.isdir(self.use_system_cxx_lib):
+self.cxx.link_flags += ['-L' + self.use_system_cxx_lib]
+if not self.is_windows:
+self.cxx.link_flags += ['-Wl,-rpath,' +
+self.use_system_cxx_lib]
+if self.is_windows and self.link_shared:
+self.add_path(self.cxx.compile_env, self.use_system_cxx_lib)
 
 def configure_link_flags_abi_library_path(self):
 # Configure ABI library paths.


Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -275,7 +275,13 @@
 # compatibility between the current headers and a shipping dynamic
 # library.
 # Default to testing against the locally built libc++ library.
-self.use_system_cxx_lib = self.get_lit_bool('use_system_cxx_lib', False)
+self.use_system_cxx_lib = self.get_lit_conf('use_system_cxx_lib')
+if self.use_system_cxx_lib == 'true':
+self.use_system_cxx_lib = True
+elif self.use_system_cxx_lib == 'false':
+self.use_system_cxx_lib = False
+else:
+assert os.path.isdir(self.use_system_cxx_lib)
 self.lit_config.note(
 "inferred use_system_cxx_lib as: %r" % self.use_system_cxx_lib)
 
@@ -719,6 +725,13 @@
 self.cxx_runtime_root]
 elif self.is_windows and self.link_shared:
 self.add_path(self.exec_env, self.cxx_runtime_root)
+elif os.path.isdir(self.use_system_cxx_lib):
+self.cxx.link_flags += ['-L' + self.use_system_cxx_lib]
+if not self.is_windows:
+self.cxx.link_flags += ['-Wl,-rpath,' +
+self.use_system_cxx_lib]
+if self.is_windows and self.link_shared:
+self.add_path(self.cxx.compile_env, self.use_system_cxx_lib)
 
 def configure_link_flags_abi_library_path(self):
 # Configure ABI library paths.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29599: Clang Changes for alloc_align

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) is at least as aligned as the value indicated parameter. The 
+parameter is given by its index in the list of formal parameters; the first

"value *of the* indicated parameter"



Comment at: include/clang/Basic/AttrDocs.td:254
+parameter is given by its index in the list of formal parameters; the first
+parameter hs index 1 unless the function is a C++ non-static member function,
+in which case the first parameter has index 2 to account for the implicit 
``this``

"has"



Comment at: lib/CodeGen/CGCall.cpp:4363
+} else if (AllocAlignParam && TargetDecl->hasAttr())
+  EmitAlignmentAssumption(Ret.getScalarVal(), AllocAlignParam);
   }

Your old code was fine, you just needed to get the value with 
CallArgs[index].second.getScalarVal() instead of IRCallArgs[index].


https://reviews.llvm.org/D29599



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

Re: [PATCH] D30388: [XRay] Add -fxray-{always,never}-instrument= flags to clang

[Dean Michael Berris via cfe-commits]

Oops, thanks -- yes, I'll make that change now.

On Thu, Mar 30, 2017 at 12:03 PM Andrew Ford via Phabricator <
revi...@reviews.llvm.org> wrote:

> andrewford added a comment.
>
> Hi, this breaks the android build due to use of std::to_string.
> llvm::to_string should be used instead.  I would fix it myself, but don't
> have commit access.  Thanks in advance!
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D30388
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r299044 - [XRay][clang] Use llvm::to_string instead of std::string

[Dean Michael Berris via cfe-commits]

Author: dberris
Date: Wed Mar 29 20:05:09 2017
New Revision: 299044

URL: http://llvm.org/viewvc/llvm-project?rev=299044&view=rev
Log:
[XRay][clang] Use llvm::to_string instead of std::string

This should unbreak some bots.

Follow-up on D30388.

Modified:
cfe/trunk/lib/Driver/XRayArgs.cpp

Modified: cfe/trunk/lib/Driver/XRayArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/XRayArgs.cpp?rev=299044&r1=299043&r2=299044&view=diff
==
--- cfe/trunk/lib/Driver/XRayArgs.cpp (original)
+++ cfe/trunk/lib/Driver/XRayArgs.cpp Wed Mar 29 20:05:09 2017
@@ -17,6 +17,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 using namespace clang::driver;
@@ -91,7 +92,7 @@ void XRayArgs::addArgs(const ToolChain &
 
   CmdArgs.push_back(XRayInstrumentOption);
   CmdArgs.push_back(Args.MakeArgString(XRayInstructionThresholdOption +
-   std::to_string(InstructionThreshold)));
+   llvm::to_string(InstructionThreshold)));
 
   for (const auto &Always : AlwaysInstrumentFiles) {
 SmallString<64> AlwaysInstrumentOpt(XRayAlwaysInstrumentOption);


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

Re: [PATCH] D30388: [XRay] Add -fxray-{always,never}-instrument= flags to clang

[Dean Michael Berris via cfe-commits]

Fix committed in r299044.

On Thu, Mar 30, 2017 at 12:12 PM Dean Michael Berris 
wrote:

> Oops, thanks -- yes, I'll make that change now.
>
> On Thu, Mar 30, 2017 at 12:03 PM Andrew Ford via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> andrewford added a comment.
>
> Hi, this breaks the android build due to use of std::to_string.
> llvm::to_string should be used instead.  I would fix it myself, but don't
> have commit access.  Thanks in advance!
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D30388
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r299045 - Use 'unsigned' for enum bitfields

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Mar 29 20:12:08 2017
New Revision: 299045

URL: http://llvm.org/viewvc/llvm-project?rev=299045&view=rev
Log:
Use 'unsigned' for enum bitfields

Fixes this clang warning on Windows:

warning: implicit truncation from 'clang::LangOptions::FPContractModeKind' to 
bit-field changes value from 2 to -2 [-Wbitfield-constant-conversion]
fp_contract = LangOptions::FPC_Fast;
^ ~


Modified:
cfe/trunk/include/clang/Basic/LangOptions.h   (contents, props changed)

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=299045&r1=299044&r2=299045&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Wed Mar 29 20:12:08 2017
@@ -224,7 +224,7 @@ public:
 
 private:
   /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this.
-  LangOptions::FPContractModeKind fp_contract : 2;
+  unsigned fp_contract : 2;
 };
 
 /// \brief Describes the kind of translation unit being processed.

Propchange: cfe/trunk/include/clang/Basic/LangOptions.h
--
--- svn:eol-style (original)
+++ svn:eol-style (removed)
@@ -1 +0,0 @@
-native


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

[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

Certain implicitly generated coroutine statements, such as the calls to 
'return_value()' or `return_void()` or 
`get_return_object_on_allocation_failure()`, cannot be built until the promise 
type is no longer dependent. This means they are not built until after the 
coroutine body statement has been transformed.

This patch fixes an issue where these statements would never be built for 
coroutine templates.

It also fixes a small issue where diagnostics about 
`get_return_object_on_allocation_failure()` were incorrectly suppressed.


https://reviews.llvm.org/D31487

Files:
  include/clang/AST/StmtCXX.h
  lib/AST/StmtCXX.cpp
  lib/Sema/CoroutineBuilder.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -534,6 +534,12 @@
   co_await a;
 }
 
+template 
+coro bad_implicit_return_dependent(T) { // expected-error {{'bad_promise_6' declares both 'return_value' and 'return_void'}}
+  co_await a;
+}
+template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}}
+
 struct bad_promise_7 {
   coro get_return_object();
   suspend_always initial_suspend();
@@ -544,25 +550,38 @@
   co_await a;
 }
 
+template 
+coro no_unhandled_exception_dependent(T) { // expected-error {{'bad_promise_7' is required to declare the member 'unhandled_exception()'}}
+  co_await a;
+}
+template coro no_unhandled_exception_dependent(bad_promise_7); // expected-note {{in instantiation}}
+
 struct bad_promise_base {
 private:
   void return_void();
 };
 struct bad_promise_8 : bad_promise_base {
   coro get_return_object();
   suspend_always initial_suspend();
   suspend_always final_suspend();
-  void unhandled_exception() __attribute__((unavailable)); // expected-note {{made unavailable}}
-  void unhandled_exception() const;// expected-note {{candidate}}
-  void unhandled_exception(void *) const;  // expected-note {{requires 1 argument, but 0 were provided}}
+  void unhandled_exception() __attribute__((unavailable)); // expected-note 2 {{made unavailable}}
+  void unhandled_exception() const;// expected-note 2 {{candidate}}
+  void unhandled_exception(void *) const;  // expected-note 2 {{requires 1 argument, but 0 were provided}}
 };
 coro calls_unhandled_exception() {
   // expected-error@-1 {{call to unavailable member function 'unhandled_exception'}}
   // FIXME: also warn about private 'return_void' here. Even though building
   // the call to unhandled_exception has already failed.
   co_await a;
 }
 
+template 
+coro calls_unhandled_exception_dependent(T) {
+  // expected-error@-1 {{call to unavailable member function 'unhandled_exception'}}
+  co_await a;
+}
+template coro calls_unhandled_exception_dependent(bad_promise_8); // expected-note {{in instantiation}}
+
 struct bad_promise_9 {
   coro get_return_object();
   suspend_always initial_suspend();
@@ -652,3 +671,26 @@
 extern "C" int f(promise_on_alloc_failure_tag) {
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
+
+struct bad_promise_11 {
+  coro get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void unhandled_exception();
+  void return_void();
+
+private:
+  static coro get_return_object_on_allocation_failure(); // expected-note 2 {{declared private here}}
+};
+coro private_alloc_failure_handler() {
+  // expected-error@-1 {{'get_return_object_on_allocation_failure' is a private member of 'bad_promise_11'}}
+  co_return; // FIXME: Add a "declared coroutine here" note.
+}
+
+template 
+coro dependent_private_alloc_failure_handler(T) {
+  // expected-error@-1 {{'get_return_object_on_allocation_failure' is a private member of 'bad_promise_11'}}
+  co_return; // FIXME: Add a "declared coroutine here" note.
+}
+template coro dependent_private_alloc_failure_handler(bad_promise_11);
+// expected-note@-1 {{requested here}}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 #define LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 
+#include "CoroutineBuilder.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
@@ -6849,11 +6850,10 @@
 TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
   // The coroutine body should be re-formed by the caller if necessary.
   // FIXME: The coroutine body is always rebuilt by ActOnFinishFunctionBody
-  CoroutineBodyStmt::CtorArgs BodyArgs;
 
   auto *ScopeInfo = SemaRef.getCurFunction();
   auto *FD = cast(SemaRef.CurContext);
-  assert(ScopeInfo && !ScopeInfo->CoroutinePromise &&
+  assert(FD && ScopeInfo

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

[Eric Fiselier via Phabricator via cfe-commits]

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



Comment at: lib/CMakeLists.txt:155
+# We can't use the "-reexported_symbols_list" when we build the
+# new/delete operators as part of the dylib: the linker would fail.
+set(OSX_RE_EXPORT_LINE 
"-Wl,-reexport_library,${CMAKE_OSX_SYSROOT}/usr/lib/libc++abi.dylib")

Please explain that the linker will fail because `libc++abi` also provides 
those definitions.


https://reviews.llvm.org/D31272



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

[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 93441.
EricWF added a comment.

- Remove unneeded asserts.


https://reviews.llvm.org/D31487

Files:
  include/clang/AST/StmtCXX.h
  lib/AST/StmtCXX.cpp
  lib/Sema/CoroutineBuilder.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -534,6 +534,12 @@
   co_await a;
 }
 
+template 
+coro bad_implicit_return_dependent(T) { // expected-error {{'bad_promise_6' declares both 'return_value' and 'return_void'}}
+  co_await a;
+}
+template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}}
+
 struct bad_promise_7 {
   coro get_return_object();
   suspend_always initial_suspend();
@@ -544,25 +550,38 @@
   co_await a;
 }
 
+template 
+coro no_unhandled_exception_dependent(T) { // expected-error {{'bad_promise_7' is required to declare the member 'unhandled_exception()'}}
+  co_await a;
+}
+template coro no_unhandled_exception_dependent(bad_promise_7); // expected-note {{in instantiation}}
+
 struct bad_promise_base {
 private:
   void return_void();
 };
 struct bad_promise_8 : bad_promise_base {
   coro get_return_object();
   suspend_always initial_suspend();
   suspend_always final_suspend();
-  void unhandled_exception() __attribute__((unavailable)); // expected-note {{made unavailable}}
-  void unhandled_exception() const;// expected-note {{candidate}}
-  void unhandled_exception(void *) const;  // expected-note {{requires 1 argument, but 0 were provided}}
+  void unhandled_exception() __attribute__((unavailable)); // expected-note 2 {{made unavailable}}
+  void unhandled_exception() const;// expected-note 2 {{candidate}}
+  void unhandled_exception(void *) const;  // expected-note 2 {{requires 1 argument, but 0 were provided}}
 };
 coro calls_unhandled_exception() {
   // expected-error@-1 {{call to unavailable member function 'unhandled_exception'}}
   // FIXME: also warn about private 'return_void' here. Even though building
   // the call to unhandled_exception has already failed.
   co_await a;
 }
 
+template 
+coro calls_unhandled_exception_dependent(T) {
+  // expected-error@-1 {{call to unavailable member function 'unhandled_exception'}}
+  co_await a;
+}
+template coro calls_unhandled_exception_dependent(bad_promise_8); // expected-note {{in instantiation}}
+
 struct bad_promise_9 {
   coro get_return_object();
   suspend_always initial_suspend();
@@ -652,3 +671,26 @@
 extern "C" int f(promise_on_alloc_failure_tag) {
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
+
+struct bad_promise_11 {
+  coro get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void unhandled_exception();
+  void return_void();
+
+private:
+  static coro get_return_object_on_allocation_failure(); // expected-note 2 {{declared private here}}
+};
+coro private_alloc_failure_handler() {
+  // expected-error@-1 {{'get_return_object_on_allocation_failure' is a private member of 'bad_promise_11'}}
+  co_return; // FIXME: Add a "declared coroutine here" note.
+}
+
+template 
+coro dependent_private_alloc_failure_handler(T) {
+  // expected-error@-1 {{'get_return_object_on_allocation_failure' is a private member of 'bad_promise_11'}}
+  co_return; // FIXME: Add a "declared coroutine here" note.
+}
+template coro dependent_private_alloc_failure_handler(bad_promise_11);
+// expected-note@-1 {{requested here}}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 #define LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 
+#include "CoroutineBuilder.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
@@ -6849,11 +6850,10 @@
 TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
   // The coroutine body should be re-formed by the caller if necessary.
   // FIXME: The coroutine body is always rebuilt by ActOnFinishFunctionBody
-  CoroutineBodyStmt::CtorArgs BodyArgs;
 
   auto *ScopeInfo = SemaRef.getCurFunction();
   auto *FD = cast(SemaRef.CurContext);
-  assert(ScopeInfo && !ScopeInfo->CoroutinePromise &&
+  assert(FD && ScopeInfo && !ScopeInfo->CoroutinePromise &&
  ScopeInfo->NeedsCoroutineSuspends &&
  ScopeInfo->CoroutineSuspends.first == nullptr &&
  ScopeInfo->CoroutineSuspends.second == nullptr &&
@@ -6865,17 +6865,11 @@
 
   // The new CoroutinePromise object needs to be built and put into the current
   // FunctionScopeInfo before any transformations or rebuilding occurs.
-  auto *Promise = S->getPromiseDecl();
-  auto *NewPromise = SemaRef.buildCoroutinePromise(F

Re: [libcxx] r281681 - [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI compatibility

[Duncan P. N. Exon Smith via cfe-commits]

Why are we propagating the use of always_inline?

In other places, we use always_inline to avoid affecting ABI (a visibility 
hack).  But you're not removing basic_string from the dylib here.

It has major downsides:
- It causes inlining at -O0, which causes major regressions in debugging 
experiences.
- It causes inlining at -O0, which, without other optimizations, can blow up 
stack size of static_initializers.
- It overrides the inliner heuristics, which *should* be smarter.

Why can't we get away with just the `inline` keyword and trust the compiler 
(and/or fix the compiler)?

> On Sep 15, 2016, at 17:00, Eric Fiselier via cfe-commits 
>  wrote:
> 
> Author: ericwf
> Date: Thu Sep 15 19:00:48 2016
> New Revision: 281681
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=281681&view=rev
> Log:
> [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI 
> compatibility
> 
> Summary:
> GCC and Clang handle visibility attributes on the out-of-line definition of 
> externally instantiated templates differently. For example in the reproducer 
> below Clang will emit both 'foo' and 'bar' with default visibility while GCC 
> only emits a non-hidden 'foo'.  
> 
> ```
> // RUN: g++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
> // RUN: clang++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
> #define INLINE_VISIBILITY __attribute__((visibility("hidden"), always_inline))
> 
> template 
> struct Foo {
>  void foo();
>  void bar();
> };
> 
> template 
> void Foo::foo() {}
> 
> template 
> inline INLINE_VISIBILITY
> void Foo::bar() {}
> 
> template struct Foo;
> ```
> 
> This difference creates ABI incompatibilities between Clang and GCC built 
> dylibs. Specifically GCC built dylibs lack definitions for various member 
> functions of `basic_string`, `basic_istream`, `basic_ostream`, 
> `basic_iostream`, and `basic_streambuf` (All of these types are externally 
> instantiated). 
> 
> Surprisingly these missing symbols don't cause many problems because the 
> functions are marked `always_inline`  therefore the dylib definition is 
> rarely needed. However when an out-of-line definition is required then GCC 
> built dylibs will fail to link. For example [GCC built dylibs cannot build 
> Clang](http://stackoverflow.com/questions/39454262/clang-build-errors).
> 
> This patch works around this issue by adding 
> `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` which is used to mark externally 
> instantiated member functions as always inline. When building the library 
> `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` sets the symbol's visibility to 
> "default" instead of "hidden", otherwise it acts exactly the same as 
> `_LIBCPP_INLINE_VISIBILITY`.
> 
> After applying this patch GCC dylibs now contain:
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE7sungetcEv`
>  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE5gbumpEi`
>  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE7sungetcEv`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE9sputbackcEc`
>  * 
> `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEE3getERNS_15basic_streambufIwS2_EE`
>  * 
> `_ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEElsEPFRNS_9basic_iosIwS2_EES6_E`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE4setpEPcS4_`
>  * 
> `_ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEEC1EPNS_15basic_streambufIwS2_EE`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE6snextcEv`
>  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEE4swapERS3_`
>  * `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEE4swapERS3_`
>  * 
> `_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE6__initEPKcm`
>  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEErsEPFRNS_8ios_baseES5_E`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE9pubsetbufEPcl`
>  * 
> `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE10pubseekoffExNS_8ios_base7seekdirEj`
>  * 
> `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEErsEPFRNS_9basic_iosIwS2_EES6_E`
>  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE5pbumpEi`
>  * 
> `_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE5seekpENS_4fposI11__mbstate_tEE`
>  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEE7getlineEPcl`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE5sgetcEv`
>  * 
> `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEE3getERNS_15basic_streambufIcS2_EE`
>  * `_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEPFRNS_8ios_baseES5_E`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE8in_availEv`
>  * `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEErsEPFRNS_8ios_baseES5_E`
>  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE6sbumpcEv`
>  * 
> `_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEPFRNS_9basic_iosIcS2_EES6_E`
>  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEE3getERc`
>  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE6snextcEv`
>  * `_ZNSt3__112basic_stringIwNS_11char_traitsIwEENS_9allocatorIwEEE6__initEmw`
>  * `_ZNSt3__113basic_istreamIwNS_11char_traitsI

Re: [libcxx] r281681 - [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI compatibility

[Eric Fiselier via cfe-commits]

On Wed, Mar 29, 2017 at 9:00 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

> Why are we propagating the use of always_inline?
>

The intent of this change wasn't to propagate or remove always_inline from
any functions. It was to fix incompatible dylibs built by GCC.
The way it did that is by removing the
__attribute__((visibility("hidden"))) part while building the dylib,
because GCC was ignoring the
`visibility("default")` on the extern template declarations for basic
string.


> In other places, we use always_inline to avoid affecting ABI (a visibility
> hack).  But you're not removing basic_string from the dylib here.
>

That's a very good point. It's likely we should convert many of the
`_LIBCPP_ALWAYS_INLINE`s to inline instead and the compiler should
do the right thing. However that change is orthogonal to this one, and
hence it wasn't made here.

Also I think the doc for _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY is
misleading. I'll try to update it in the coming days.



> It has major downsides:
> - It causes inlining at -O0, which causes major regressions in debugging
> experiences.
> - It causes inlining at -O0, which, without other optimizations, can blow
> up stack size of static_initializers.
> - It overrides the inliner heuristics, which *should* be smarter.
>
> Why can't we get away with just the `inline` keyword and trust the
> compiler (and/or fix the compiler)?
>
> > On Sep 15, 2016, at 17:00, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: ericwf
> > Date: Thu Sep 15 19:00:48 2016
> > New Revision: 281681
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=281681&view=rev
> > Log:
> > [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC
> ABI compatibility
> >
> > Summary:
> > GCC and Clang handle visibility attributes on the out-of-line definition
> of externally instantiated templates differently. For example in the
> reproducer below Clang will emit both 'foo' and 'bar' with default
> visibility while GCC only emits a non-hidden 'foo'.
> >
> > ```
> > // RUN: g++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
> > // RUN: clang++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
> > #define INLINE_VISIBILITY __attribute__((visibility("hidden"),
> always_inline))
> >
> > template 
> > struct Foo {
> >  void foo();
> >  void bar();
> > };
> >
> > template 
> > void Foo::foo() {}
> >
> > template 
> > inline INLINE_VISIBILITY
> > void Foo::bar() {}
> >
> > template struct Foo;
> > ```
> >
> > This difference creates ABI incompatibilities between Clang and GCC
> built dylibs. Specifically GCC built dylibs lack definitions for various
> member functions of `basic_string`, `basic_istream`, `basic_ostream`,
> `basic_iostream`, and `basic_streambuf` (All of these types are externally
> instantiated).
> >
> > Surprisingly these missing symbols don't cause many problems because the
> functions are marked `always_inline`  therefore the dylib definition is
> rarely needed. However when an out-of-line definition is required then GCC
> built dylibs will fail to link. For example [GCC built dylibs cannot build
> Clang](http://stackoverflow.com/questions/39454262/clang-build-errors).
> >
> > This patch works around this issue by adding 
> > `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY`
> which is used to mark externally instantiated member functions as always
> inline. When building the library `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY`
> sets the symbol's visibility to "default" instead of "hidden", otherwise it
> acts exactly the same as `_LIBCPP_INLINE_VISIBILITY`.
> >
> > After applying this patch GCC dylibs now contain:
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE7sungetcEv`
> >  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE5gbumpEi`
> >  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE7sungetcEv`
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE9sputbackcEc`
> >  * `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEE3getERNS_
> 15basic_streambufIwS2_EE`
> >  * `_ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEElsEPFRNS_
> 9basic_iosIwS2_EES6_E`
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE4setpEPcS4_`
> >  * `_ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEEC1EPNS_
> 15basic_streambufIwS2_EE`
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE6snextcEv`
> >  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEE4swapERS3_`
> >  * `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEE4swapERS3_`
> >  * `_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_
> 9allocatorIcEEE6__initEPKcm`
> >  * `_ZNSt3__113basic_istreamIcNS_11char_traitsIcEEErsEPFRNS_
> 8ios_baseES5_E`
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE9pubsetbufEPcl`
> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE10pubseekoffExNS_
> 8ios_base7seekdirEj`
> >  * `_ZNSt3__113basic_istreamIwNS_11char_traitsIwEEErsEPFRNS_
> 9basic_iosIwS2_EES6_E`
> >  * `_ZNSt3__115basic_streambufIwNS_11char_traitsIwEEE5pbumpEi`

[PATCH] D31167: Use FPContractModeKind universally

[Adam Nemet via Phabricator via cfe-commits]

anemet added inline comments.



Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217
   /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this.
-  unsigned fp_contract : 1;
+  LangOptions::FPContractModeKind fp_contract : 2;
 };

rnk wrote:
> Please do not use bitfields with enum types, it's a good way to break the 
> build on Windows. This change triggered this clang-cl warning:
> ```
> C:\src\llvm-project\clang\include\clang/Basic/LangOptions.h(208,17):  
> warning: implicit truncation from 'clang::LangOptions::FPContractModeKind' to 
> bit-field changes value from 2 to -2 [-Wbitfield-constant-conversion]
> fp_contract = LangOptions::FPC_Fast;
> ^ ~
> ```
Noted and thanks for the fix!  Unfortunately the warning wasn't showing up on 
my host.  I'll take a look why.


Repository:
  rL LLVM

https://reviews.llvm.org/D31167



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

[PATCH] D31486: libc++ testing: allow to provide a path for `use_system_cxx_lib`

[Eric Fiselier via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D31486



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

Re: [libcxx] r281681 - [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI compatibility

[Eric Fiselier via cfe-commits]

On Wed, Mar 29, 2017 at 9:30 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

>
> On Mar 29, 2017, at 20:16, Eric Fiselier  wrote:
>
>
>
> On Wed, Mar 29, 2017 at 9:00 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>> Why are we propagating the use of always_inline?
>>
>
> The intent of this change wasn't to propagate or remove always_inline from
> any functions. It was to fix incompatible dylibs built by GCC.
> The way it did that is by removing the __attribute__((visibility("hidden")))
> part while building the dylib, because GCC was ignoring the
> `visibility("default")` on the extern template declarations for basic
> string.
>
>
>> In other places, we use always_inline to avoid affecting ABI (a
>> visibility hack).  But you're not removing basic_string from the dylib here.
>>
>
> That's a very good point. It's likely we should convert many of the
> `_LIBCPP_ALWAYS_INLINE`s to inline instead and the compiler should
> do the right thing. However that change is orthogonal to this one, and
> hence it wasn't made here.
>
>
> Reading comprehension issue on my part.  I think probably replied to the
> wrong commit.  Bisection pointed at r278356, but I saw that that commit was
> reverted and I couldn't find where it finally landed.  So then I did a
> careless git-blame for what I thought was the relevant line and ended up
> here.
>

Actually upon further inspection you may be correct to blame this commit.
This change also moved the attributes to the first declaration from the
out-of-line definition. This seems to affect if Clang emits the "hidden"
part. Previously it did not, but after moving the declarations it seems to.

>
> Also I think the doc for _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY is
> misleading. I'll try to update it in the coming days.
>
>
>
>
>> It has major downsides:
>> - It causes inlining at -O0, which causes major regressions in debugging
>> experiences.
>> - It causes inlining at -O0, which, without other optimizations, can blow
>> up stack size of static_initializers.
>>
>
> For some extra context: we tracked down a stack overflow to the related
> changes to add always_inline to basic_string::__init.  Our user had some
> string => enum value map declared statically, like in the attached
> stackoverflow.cpp.  At -Os (and higher), there's no difference.  But with
> -O0 and -O1, they started getting a stack overflow at launch.
>
>
>
> - It overrides the inliner heuristics, which *should* be smarter.
>>
>> Why can't we get away with just the `inline` keyword and trust the
>> compiler (and/or fix the compiler)?
>>
>> > On Sep 15, 2016, at 17:00, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Author: ericwf
>> > Date: Thu Sep 15 19:00:48 2016
>> > New Revision: 281681
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=281681&view=rev
>> > Log:
>> > [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC
>> ABI compatibility
>> >
>> > Summary:
>> > GCC and Clang handle visibility attributes on the out-of-line
>> definition of externally instantiated templates differently. For example in
>> the reproducer below Clang will emit both 'foo' and 'bar' with default
>> visibility while GCC only emits a non-hidden 'foo'.
>> >
>> > ```
>> > // RUN: g++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
>> > // RUN: clang++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
>> > #define INLINE_VISIBILITY __attribute__((visibility("hidden"),
>> always_inline))
>> >
>> > template 
>> > struct Foo {
>> >  void foo();
>> >  void bar();
>> > };
>> >
>> > template 
>> > void Foo::foo() {}
>> >
>> > template 
>> > inline INLINE_VISIBILITY
>> > void Foo::bar() {}
>> >
>> > template struct Foo;
>> > ```
>> >
>> > This difference creates ABI incompatibilities between Clang and GCC
>> built dylibs. Specifically GCC built dylibs lack definitions for various
>> member functions of `basic_string`, `basic_istream`, `basic_ostream`,
>> `basic_iostream`, and `basic_streambuf` (All of these types are externally
>> instantiated).
>> >
>> > Surprisingly these missing symbols don't cause many problems because
>> the functions are marked `always_inline`  therefore the dylib definition is
>> rarely needed. However when an out-of-line definition is required then GCC
>> built dylibs will fail to link. For example [GCC built dylibs cannot build
>> Clang](http://stackoverflow.com/questions/39454262/clang-build-errors).
>> >
>> > This patch works around this issue by adding
>> `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` which is used to mark
>> externally instantiated member functions as always inline. When building
>> the library `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` sets the
>> symbol's visibility to "default" instead of "hidden", otherwise it acts
>> exactly the same as `_LIBCPP_INLINE_VISIBILITY`.
>> >
>> > After applying this patch GCC dylibs now contain:
>> >  * `_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE7sungetcEv`
>> >  * `_Z

Re: [libcxx] r281681 - [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI compatibility

[Mehdi AMINI via cfe-commits]

Hi,

I don't understand clang's logic here, seems like a bug to me. I changed
slightly the test-case, and I'm wondering why only foo() is emitted as
hidden in the following:

#define INLINE_VISIBILITY __attribute__((visibility("hidden"),
always_inline))

template 
struct Foo {
 void INLINE_VISIBILITY foo();
 void bar();
};

template 
inline
void Foo::foo() {}

template 
inline INLINE_VISIBILITY
void Foo::bar() {}

void odr_use() {
  Foo().bar();
  Foo().foo();
}


See here: https://godbolt.org/g/FLBnu7

-- 
Mehdi


2017-03-29 20:36 GMT-07:00 Eric Fiselier :

>
>
> On Wed, Mar 29, 2017 at 9:30 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>>
>> On Mar 29, 2017, at 20:16, Eric Fiselier  wrote:
>>
>>
>>
>> On Wed, Mar 29, 2017 at 9:00 PM, Duncan P. N. Exon Smith <
>> dexonsm...@apple.com> wrote:
>>
>>> Why are we propagating the use of always_inline?
>>>
>>
>> The intent of this change wasn't to propagate or remove always_inline
>> from any functions. It was to fix incompatible dylibs built by GCC.
>> The way it did that is by removing the __attribute__((visibility("hidden")))
>> part while building the dylib, because GCC was ignoring the
>> `visibility("default")` on the extern template declarations for basic
>> string.
>>
>>
>>> In other places, we use always_inline to avoid affecting ABI (a
>>> visibility hack).  But you're not removing basic_string from the dylib here.
>>>
>>
>> That's a very good point. It's likely we should convert many of the
>> `_LIBCPP_ALWAYS_INLINE`s to inline instead and the compiler should
>> do the right thing. However that change is orthogonal to this one, and
>> hence it wasn't made here.
>>
>>
>> Reading comprehension issue on my part.  I think probably replied to the
>> wrong commit.  Bisection pointed at r278356, but I saw that that commit was
>> reverted and I couldn't find where it finally landed.  So then I did a
>> careless git-blame for what I thought was the relevant line and ended up
>> here.
>>
>
> Actually upon further inspection you may be correct to blame this commit.
> This change also moved the attributes to the first declaration from the
> out-of-line definition. This seems to affect if Clang emits the "hidden"
> part. Previously it did not, but after moving the declarations it seems to.
>
>>
>> Also I think the doc for _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY is
>> misleading. I'll try to update it in the coming days.
>>
>>
>>
>>
>>> It has major downsides:
>>> - It causes inlining at -O0, which causes major regressions in debugging
>>> experiences.
>>> - It causes inlining at -O0, which, without other optimizations, can
>>> blow up stack size of static_initializers.
>>>
>>
>> For some extra context: we tracked down a stack overflow to the related
>> changes to add always_inline to basic_string::__init.  Our user had some
>> string => enum value map declared statically, like in the attached
>> stackoverflow.cpp.  At -Os (and higher), there's no difference.  But with
>> -O0 and -O1, they started getting a stack overflow at launch.
>>
>>
>>
>> - It overrides the inliner heuristics, which *should* be smarter.
>>>
>>> Why can't we get away with just the `inline` keyword and trust the
>>> compiler (and/or fix the compiler)?
>>>
>>> > On Sep 15, 2016, at 17:00, Eric Fiselier via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Author: ericwf
>>> > Date: Thu Sep 15 19:00:48 2016
>>> > New Revision: 281681
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=281681&view=rev
>>> > Log:
>>> > [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC
>>> ABI compatibility
>>> >
>>> > Summary:
>>> > GCC and Clang handle visibility attributes on the out-of-line
>>> definition of externally instantiated templates differently. For example in
>>> the reproducer below Clang will emit both 'foo' and 'bar' with default
>>> visibility while GCC only emits a non-hidden 'foo'.
>>> >
>>> > ```
>>> > // RUN: g++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
>>> > // RUN: clang++ -std=c++11 -shared -O3 test.cpp && sym_extract.py a.out
>>> > #define INLINE_VISIBILITY __attribute__((visibility("hidden"),
>>> always_inline))
>>> >
>>> > template 
>>> > struct Foo {
>>> >  void foo();
>>> >  void bar();
>>> > };
>>> >
>>> > template 
>>> > void Foo::foo() {}
>>> >
>>> > template 
>>> > inline INLINE_VISIBILITY
>>> > void Foo::bar() {}
>>> >
>>> > template struct Foo;
>>> > ```
>>> >
>>> > This difference creates ABI incompatibilities between Clang and GCC
>>> built dylibs. Specifically GCC built dylibs lack definitions for various
>>> member functions of `basic_string`, `basic_istream`, `basic_ostream`,
>>> `basic_iostream`, and `basic_streambuf` (All of these types are externally
>>> instantiated).
>>> >
>>> > Surprisingly these missing symbols don't cause many problems because
>>> the functions are marked `always_inline`  therefore the dylib definition is
>>> rarely needed. However when an out-of-line definition is required the

Re: [libcxx] r281681 - [libc++] Add _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY to support GCC ABI compatibility

[Mehdi AMINI via cfe-commits]

2017-03-29 20:30 GMT-07:00 Duncan P. N. Exon Smith :

>
> On Mar 29, 2017, at 20:16, Eric Fiselier  wrote:
>
>
>
> On Wed, Mar 29, 2017 at 9:00 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>> Why are we propagating the use of always_inline?
>>
>
> The intent of this change wasn't to propagate or remove always_inline from
> any functions. It was to fix incompatible dylibs built by GCC.
> The way it did that is by removing the __attribute__((visibility("hidden")))
> part while building the dylib, because GCC was ignoring the
> `visibility("default")` on the extern template declarations for basic
> string.
>
>
>> In other places, we use always_inline to avoid affecting ABI (a
>> visibility hack).  But you're not removing basic_string from the dylib here.
>>
>
> That's a very good point. It's likely we should convert many of the
> `_LIBCPP_ALWAYS_INLINE`s to inline instead and the compiler should
> do the right thing. However that change is orthogonal to this one, and
> hence it wasn't made here.
>
>
> Reading comprehension issue on my part.  I think probably replied to the
> wrong commit.  Bisection pointed at r278356, but I saw that that commit was
> reverted and I couldn't find where it finally landed.  So then I did a
> careless git-blame for what I thought was the relevant line and ended up
> here.
>
> Also I think the doc for _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY is
> misleading. I'll try to update it in the coming days.
>
>
>
>
>> It has major downsides:
>> - It causes inlining at -O0, which causes major regressions in debugging
>> experiences.
>> - It causes inlining at -O0, which, without other optimizations, can blow
>> up stack size of static_initializers.
>>
>
> For some extra context: we tracked down a stack overflow to the related
> changes to add always_inline to basic_string::__init.  Our user had some
> string => enum value map declared statically, like in the attached
> stackoverflow.cpp.  At -Os (and higher), there's no difference.  But with
> -O0 and -O1, they started getting a stack overflow at launch.
>

That's not a bug but a feature ;-)
It was a good way to show the user he's not doing the right thing by using
a costly initialization routine!

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

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: lib/CMakeLists.txt:155
+# We can't use the "-reexported_symbols_list" when we build the
+# new/delete operators as part of the dylib: the linker would fail.
+set(OSX_RE_EXPORT_LINE 
"-Wl,-reexport_library,${CMAKE_OSX_SYSROOT}/usr/lib/libc++abi.dylib")

EricWF wrote:
> Please explain that the linker will fail because `libc++abi` also provides 
> those definitions.
Mmmm, actually no, the linker fails because libc++ provide those definition.
The linker complains because we can't ask to export the definitions from 
libc++abi while we already have some definitions!


https://reviews.llvm.org/D31272



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