RE: [PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

[Liu, Yaxun (Sam) via cfe-commits]

I will simplify the test. Thanks.

Sam

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, October 23, 2017 2:08 PM
To: reviews+d39069+public+8da79e110d303...@reviews.llvm.org; Yaxun Liu via 
Phabricator ; Liu, Yaxun (Sam) ; 
rjmcc...@gmail.com
Cc: cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

What John said, but also a narrower test would be good - checking that the 
appropriate call instruction gets a debug location, rather than checking that a 
bunch of inlining doesn't cause the assertion/verifier failure, would be good. 
(Clang tests should, as much as possible, not rely on or run the LLVM 
optimization passes - but check the IR coming directly from Clang before any of 
that)
On Wed, Oct 18, 2017 at 2:15 PM Yaxun Liu via Phabricator via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
yaxunl created this revision.

Builder save/restores insertion pointer when emitting addr space cast
for alloca, but does not save/restore debug loc, which causes verifier
failure for certain call instructions.

This patch fixes that.


https://reviews.llvm.org/D39069

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenOpenCL/func-call-dbg-loc.cl


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited 
-dwarf-version=2 -debugger-tuning=gdb -O0 -emit-llvm -o - %s | FileCheck %s
+// Checks the file compiles without verifier error: inlinable function call in 
a function with debug info must have a !dbg location.
+
+typedef struct
+{
+float m_max;
+} Struct;
+
+typedef struct
+{
+Struct m_volume;
+} Node;
+
+
+Struct buzz(Node node)
+{
+return node.m_volume;
+}
+
+__attribute__((always_inline))
+float bar(Struct aabb)
+{
+return 0.0f;
+}
+
+__attribute__((used))
+void foo()
+{
+Node node;
+// CHECK: store float 0.00e+00, float addrspace(5)* %f, align 4, !dbg 
!{{[0-9]+}}
+float f = bar(buzz(node));
+}
+
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -75,11 +75,13 @@
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
 auto CurIP = Builder.saveIP();
+auto DbgLoc = Builder.getCurrentDebugLocation();
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
 Builder.restoreIP(CurIP);
+Builder.SetCurrentDebugLocation(DbgLoc);
   }

   return Address(V, Align);


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

RE: r337540 - Sema: Fix explicit address space cast in C++

[Liu, Yaxun (Sam) via cfe-commits]

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, July 20, 2018 3:29 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits 
Subject: Re: r337540 - Sema: Fix explicit address space cast in C++

On Fri, 20 Jul 2018 at 12:00, Richard Smith 
mailto:rich...@metafoo.co.uk>> wrote:
On Fri, 20 Jul 2018 at 04:38, Yaxun Liu via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: yaxunl
Date: Fri Jul 20 04:32:51 2018
New Revision: 337540

URL: http://llvm.org/viewvc/llvm-project?rev=337540&view=rev
Log:
Sema: Fix explicit address space cast in C++

Currently clang does not allow implicit cast of a pointer to a pointer type
in different address space but allows C-style cast of a pointer to a pointer
type in different address space. However, there is a bug in Sema causing
incorrect Cast Expr in AST for the latter case, which in turn results in
invalid LLVM IR in codegen.

This is because Sema::IsQualificationConversion returns true for a cast of
pointer to a pointer type in different address space, which in turn allows
a standard conversion and results in a cast expression with no op in AST.

This patch fixes that by let Sema::IsQualificationConversion returns false
for a cast of pointer to a pointer type in different address space, which
in turn disallows standard conversion, implicit cast, and static cast.
Finally it results in an reinterpret cast and correct conversion kind is set.

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

Added:
cfe/trunk/test/CodeGenCXX/address-space-cast.cpp
Modified:
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=337540&r1=337539&r2=337540&view=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Fri Jul 20 04:32:51 2018
@@ -1955,6 +1955,12 @@ static bool fixOverloadedReinterpretCast
   return Result.isUsable();
 }

+static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
+  return SrcType->isPointerType() && DestType->isPointerType() &&
+ SrcType->getAs()->getPointeeType().getAddressSpace() !=
+ 
DestType->getAs()->getPointeeType().getAddressSpace();
+}
+
 static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
 QualType DestType, bool CStyle,
 SourceRange OpRange,
@@ -2198,6 +2204,8 @@ static TryCastResult TryReinterpretCast(
 } else {
   Kind = CK_BitCast;
 }
+  } else if (IsAddressSpaceConversion(SrcType, DestType)) {
+Kind = CK_AddressSpaceConversion;

This seems wrong to me. A reinterpret_cast from a pointer to one address space 
into a pointer to a different address space should be a bit cast, not an 
address space conversion.

Hmm. I suppose we have a choice: either reinterpret_cast always means 
'reinterpret these bits as this other type' (and there are C-style casts that 
cannot be expressed in terms of named casts, violating the usual C++ rules for 
C-style casts) or reinterpret_cast between pointers always gives you a pointer 
to the same byte of storage and an address space bitcast requires casting via 
an integral type (violating the normal behavior that reinterpret_cast 
reinterprets the representation and doesn't change the value). These options 
both seem unsatisfying, but what you have here (reinterpret_cast attempts to 
form a pointer to the same byte) is definitely the more useful of those two 
options.

Do any of our supported language extensions actually say what named casts in 
C++ do when attempting to cast between address spaces? The OpenCL 2.2 C++ 
extensions specification doesn't add address space qualifiers, so it doesn't 
have an answer for us.

[Sam] I don’t know C++ based languages having rules about pointers casting to a 
different address space. OpenCL C’s rules about C-style cast of pointers to a 
different address space clearly means pointing to the same memory location. 
Bitwise cast isn’t useful in most use cases. Since there is no language rule in 
C++ for casting a pointer to a different address space, the C-style cast cannot 
be treated as a static cast in C++ since it mostly involves transformations, 
then it is more suitably treated as a reinterpret cast. If users do need 
bitwise cast, they can go through some integer, as you suggested.

   } else {
 Kind = CK_BitCast;
   }

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=337540&r1=337539&r2=337540&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Jul 20 04:32:51 2018
@@ -3150,6 +3150,15 @@ Sema::IsQualificationConversion(QualType
   = Previo

RE: r326946 - CodeGen: Fix address space of indirect function argument

[Liu, Yaxun (Sam) via cfe-commits]

I will try implementing John's suggestions. Thanks.

Sam

From: rjmcc...@apple.com [mailto:rjmcc...@apple.com]
Sent: Saturday, March 10, 2018 12:14 PM
To: Richard Smith 
Cc: Liu, Yaxun (Sam) ; cfe-commits 

Subject: Re: r326946 - CodeGen: Fix address space of indirect function argument

On Mar 9, 2018, at 8:51 PM, Richard Smith 
mailto:rich...@metafoo.co.uk>> wrote:

Hi,

This change increases the size of a CallArg, and thus that of a CallArgList, 
dramatically (an LValue is *much* larger than an RValue, and unlike an RValue, 
does not appear to be at all optimized for size). This results in CallArgList 
(which contains inline storage for 16 CallArgs) ballooning in size from ~500 
bytes to 2KB, resulting in stack overflows on programs with deep ASTs.

Given that this introduces a regression (due to stack overflow), I've reverted 
in r327195. Sorry about that.

Seems reasonable.

The right short-term fix is probably a combination of the following:
  - put a little bit of effort into packing LValue (using fewer than 64 bits 
for the alignment and packing the bit-fields, I think)
  - drop the inline argument count to something like 8, which should still 
capture the vast majority of calls.

There are quite a few other space optimizations we could do the LValue 
afterwards.   I think we could eliminate BaseIVarExp completely by storing some 
sort of ObjC GC classification and then just evaluating the base expression 
normally.


The program it broke looked something like this:

struct VectorBuilder {
  VectorBuilder &operator,(int);
};
void f() {
  VectorBuilder(),
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
  
1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0;
}

using Eigen's somewhat-ridiculous comma initializer technique 
(https://eigen.tuxfamily.org/dox/group__TutorialAdvancedInitialization.html) to 
build an approx 40 x 40 array.

An AST 1600 levels deep is somewhat extreme, but it still seems like something 
we should be able to handle within our default 8MB stack limit.

I mean, at some point this really is not supportable, even if it happens to 
build on current compilers.  If we actually documented and enforced our 
implementation limits like we're supposed to, I do not think we would allow 
this much call nesting.

John.



On 7 March 2018 at 13:45, Yaxun Liu via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: yaxunl
Date: Wed Mar  7 13:45:40 2

RE: [PATCH] D62483: [CUDA][HIP] Emit dependent libs for host only

[Liu, Yaxun (Sam) via cfe-commits]

Fixed by 361905.

Sam

-Original Message-
From: Petr Hosek via Phabricator  
Sent: Tuesday, May 28, 2019 9:54 PM
To: Liu, Yaxun (Sam) ; t...@google.com
Cc: pho...@google.com; cfe-commits@lists.llvm.org; mlek...@skidmore.edu; 
blitzrak...@gmail.com; shen...@google.com; jle...@google.com; 
zjturner0...@gmail.com
Subject: [PATCH] D62483: [CUDA][HIP] Emit dependent libs for host only

[CAUTION: External Email]

phosek added a comment.

This seems to be failing on macOS bots with the following error:

  FAIL: Clang :: CodeGenCUDA/dependent-libs.cu (3052 of 14933)
   TEST 'Clang :: CodeGenCUDA/dependent-libs.cu' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/lib/clang/9.0.0/include 
-nostdsysteminc -emit-llvm -o - -fcuda-is-device -x hip 
/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu | 
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/bin/FileCheck 
--check-prefix=DEV 
/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu
  : 'RUN: at line 2';   
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/lib/clang/9.0.0/include 
-nostdsysteminc -emit-llvm -o - -x hip 
/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu | 
/b/s/w/ir/k/recipe_cleanup/clangAoYUvt/llvm_build_dir/bin/FileCheck 
--check-prefix=HOST 
/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu
  --
  Exit Code: 1

  Command Output (stderr):
  --
  /b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu:5:10: 
error: HOST: expected string not found in input
  // HOST: llvm.dependent-libraries
   ^
  :1:1: note: scanning from here
  ; ModuleID = 
'/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu'
  ^
  :1:58: note: possible intended match here
  ; ModuleID = 
'/b/s/w/ir/k/llvm-project/clang/test/CodeGenCUDA/dependent-libs.cu'
   ^

  --

The full test log is here: 
https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8912171419117598448/+/steps/clang/0/steps/test/0/stdout


Repository:
  rC Clang

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

https://reviews.llvm.org/D62483



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

RE: [clang] b670ab7 - recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

The issue is addressed by https://reviews.llvm.org/D77028

Sam

-Original Message-
From: Joerg Sonnenberger 
Sent: Sunday, April 5, 2020 9:48 AM
To: Liu, Yaxun (Sam) ; Yaxun Liu 
Cc: cfe-commits@lists.llvm.org
Subject: Re: [clang] b670ab7 - recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit 
deferred diagnostics by a post-parsing AST travese

[CAUTION: External Email]

On Mon, Mar 23, 2020 at 09:09:31AM -0700, Yaxun Liu via cfe-commits wrote:
>
> Author: Yaxun (Sam) Liu
> Date: 2020-03-23T12:09:07-04:00
> New Revision: b670ab7b6b3d2f26179213be1da1d4ba376f50a3
>
> URL: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fb670ab7b6b3d2f26179213be1da1d4
> ba376f50a3&data=02%7C01%7Cyaxun.liu%40amd.com%7Cfc099ddf95594ab17e
> da08d7d968004e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6372169134
> 68808969&sdata=WtAm5JbQgR7UDY0FL5AlIvZOSOLPbQTRbKHJvP0Vot8%3D&
> reserved=0
> DIFF: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fb670ab7b6b3d2f26179213be1da1d4
> ba376f50a3.diff&data=02%7C01%7Cyaxun.liu%40amd.com%7Cfc099ddf95594
> ab17eda08d7d968004e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63721
> 6913468808969&sdata=wREjtZ%2F5SHC1U4dxGbbSvYYncnhqa%2ByJzHBxv%2FIh
> PlY%3D&reserved=0
>
> LOG: recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit deferred 
> diagnostics by a post-parsing AST travese

This change is responsible for a significant performance regression.
Somewhat reduced example is attached. With -fopenmp, it needs ~5s, without 
0.02s.

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

RE: [clang] b670ab7 - recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

commit 2c31aa2de13a23a00ced87123b92e905f2929c7b should fix this.

Thanks.

Sam

-Original Message-
From: Liu, Yaxun (Sam) 
Sent: Sunday, April 5, 2020 12:20 PM
To: Joerg Sonnenberger ; Yaxun Liu 
Cc: cfe-commits@lists.llvm.org
Subject: RE: [clang] b670ab7 - recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit 
deferred diagnostics by a post-parsing AST travese

[AMD Official Use Only - Internal Distribution Only]

The issue is addressed by https://reviews.llvm.org/D77028

Sam

-Original Message-
From: Joerg Sonnenberger 
Sent: Sunday, April 5, 2020 9:48 AM
To: Liu, Yaxun (Sam) ; Yaxun Liu 
Cc: cfe-commits@lists.llvm.org
Subject: Re: [clang] b670ab7 - recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit 
deferred diagnostics by a post-parsing AST travese

[CAUTION: External Email]

On Mon, Mar 23, 2020 at 09:09:31AM -0700, Yaxun Liu via cfe-commits wrote:
>
> Author: Yaxun (Sam) Liu
> Date: 2020-03-23T12:09:07-04:00
> New Revision: b670ab7b6b3d2f26179213be1da1d4ba376f50a3
>
> URL: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fb670ab7b6b3d2f26179213be1da1d4
> ba376f50a3&data=02%7C01%7Cyaxun.liu%40amd.com%7Cfc099ddf95594ab17e
> da08d7d968004e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6372169134
> 68808969&sdata=WtAm5JbQgR7UDY0FL5AlIvZOSOLPbQTRbKHJvP0Vot8%3D&
> reserved=0
> DIFF: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fb670ab7b6b3d2f26179213be1da1d4
> ba376f50a3.diff&data=02%7C01%7Cyaxun.liu%40amd.com%7Cfc099ddf95594
> ab17eda08d7d968004e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63721
> 6913468808969&sdata=wREjtZ%2F5SHC1U4dxGbbSvYYncnhqa%2ByJzHBxv%2FIh
> PlY%3D&reserved=0
>
> LOG: recommit 1b978ddba05c [CUDA][HIP][OpenMP] Emit deferred 
> diagnostics by a post-parsing AST travese

This change is responsible for a significant performance regression.
Somewhat reduced example is attached. With -fopenmp, it needs ~5s, without 
0.02s.

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

RE: [clang] 829d14e - Revert "[NFC] Refactor DiagnosticBuilder and PartialDiagnostic"

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Public Use]

It was reverted because it caused regressions in some CUDA apps.

Is there a way for me to modify the commit message?

Thanks.

Sam

-Original Message-
From: Roman Lebedev  
Sent: Thursday, September 17, 2020 2:14 PM
To: Liu, Yaxun (Sam) ; Yaxun Liu 
Cc: cfe-commits@lists.llvm.org
Subject: Re: [clang] 829d14e - Revert "[NFC] Refactor DiagnosticBuilder and 
PartialDiagnostic"

[CAUTION: External Email]

On Thu, Sep 17, 2020 at 8:57 PM Yaxun Liu via cfe-commits 
 wrote:
>
>
> Author: Yaxun (Sam) Liu
> Date: 2020-09-17T13:56:09-04:00
> New Revision: 829d14ee0a6aa79c89f7f3d9fcd9d27d3efd2b91
>
> URL: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2F829d14ee0a6aa79c89f7f3d9fcd9d2
> 7d3efd2b91&data=02%7C01%7Cyaxun.liu%40amd.com%7C1165c7b467ad471f37
> 1b08d85b358dcf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373596328
> 68922532&sdata=ngtA%2BYzoY8lfE1krECrmAw%2FXPytw2f8Nz%2FIqoyccX14%3
> D&reserved=0
> DIFF: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fllvm%2Fllvm-project%2Fcommit%2F829d14ee0a6aa79c89f7f3d9fcd9d2
> 7d3efd2b91.diff&data=02%7C01%7Cyaxun.liu%40amd.com%7C1165c7b467ad4
> 71f371b08d85b358dcf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63735
> 9632868922532&sdata=sJJczip6ORLkxglLBk48QiHbZkyrZ1uiKupti6H2ZPE%3D
> &reserved=0
>
> LOG: Revert "[NFC] Refactor DiagnosticBuilder and PartialDiagnostic"
>
> This reverts commit ee5519d323571c4a9a7d92cb817023c9b95334cd.

It is a really good idea for a commit message to actually explain what the 
commit does, not just state what it is.

> Added:
>
>
> Modified:
> clang/include/clang/AST/ASTContext.h
> clang/include/clang/AST/Attr.h
> clang/include/clang/AST/CanonicalType.h
> clang/include/clang/AST/Decl.h
> clang/include/clang/AST/DeclCXX.h
> clang/include/clang/AST/DeclarationName.h
> clang/include/clang/AST/NestedNameSpecifier.h
> clang/include/clang/AST/TemplateBase.h
> clang/include/clang/AST/TemplateName.h
> clang/include/clang/AST/Type.h
> clang/include/clang/Basic/Diagnostic.h
> clang/include/clang/Basic/PartialDiagnostic.h
> clang/include/clang/Sema/Ownership.h
> clang/include/clang/Sema/ParsedAttr.h
> clang/include/clang/Sema/Sema.h
> clang/lib/AST/ASTContext.cpp
> clang/lib/AST/DeclCXX.cpp
> clang/lib/AST/TemplateBase.cpp
> clang/lib/AST/TemplateName.cpp
> clang/lib/Basic/Diagnostic.cpp
>
> Removed:
>
>
>
> ##
> ## diff  --git a/clang/include/clang/AST/ASTContext.h 
> b/clang/include/clang/AST/ASTContext.h
> index 397fee4d866b..de0d1198b6d4 100644
> --- a/clang/include/clang/AST/ASTContext.h
> +++ b/clang/include/clang/AST/ASTContext.h
> @@ -3064,9 +3064,8 @@ OPT_LIST(V)
>  };
>
>  /// Insertion operator for diagnostics.
> -const StreamableDiagnosticBase &
> -operator<<(const StreamableDiagnosticBase &DB,
> -   const ASTContext::SectionInfo &Section);
> +const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
> +const ASTContext::SectionInfo 
> +&Section);
>
>  /// Utility function for constructing a nullary selector.
>  inline Selector GetNullarySelector(StringRef name, ASTContext &Ctx) {
>
> diff  --git a/clang/include/clang/AST/Attr.h 
> b/clang/include/clang/AST/Attr.h index b4dce8f41c67..b3729b2e0d99 
> 100644
> --- a/clang/include/clang/AST/Attr.h
> +++ b/clang/include/clang/AST/Attr.h
> @@ -350,12 +350,19 @@ struct ParsedTargetAttr {
>
>  #include "clang/AST/Attrs.inc"
>
> -inline const StreamableDiagnosticBase & -operator<<(const 
> StreamableDiagnosticBase &DB, const Attr *At) {
> +inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
> +   const Attr *At) {
>DB.AddTaggedVal(reinterpret_cast(At),
>DiagnosticsEngine::ak_attr);
>return DB;
>  }
> +
> +inline const PartialDiagnostic &operator<<(const PartialDiagnostic &PD,
> +   const Attr *At) {
> +  PD.AddTaggedVal(reinterpret_cast(At),
> +  DiagnosticsEngine::ak_attr);
> +  return PD;
> +}
>  }  // end namespace clang
>
>  #endif
>
> diff  --git a/clang/include/clang/AST/CanonicalType.h 
> b/clang/include/clang/AST/CanonicalType.h
> index b6d9b69db09a..488284713bce 100644
> --- a/clang/include/clang/AST/CanonicalType.h
> +++ b/clang/include/clang/AST/CanonicalType.h
> @@ -215,8 +215,8 @@ inline CanQualType Type::getCanonicalTypeUnqualified() 
> const {
>return CanQualType::CreateUnsafe(getCanonicalTypeInternal());
>  }
>
> -inline const StreamableDiagnosticBase & -operator<<(const 
> StreamableDiagnosticBase &DB, CanQualType T) {
> +inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
> +   CanQualType T) {
>DB << static_cast(T);
>return DB;
>  }
>

RE: [PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - General]

Fixed by 503e02e861662ef84940fdc05e26c12fc0fca7f3. Thanks.

Sam

-Original Message-
From: Nico Weber via Phabricator 
Sent: Wednesday, January 11, 2023 11:25 PM
To: Liu, Yaxun (Sam) ; t...@google.com
Cc: tha...@chromium.org; mask...@google.com; cfe-commits@lists.llvm.org; 
473750...@qq.com; tul...@redhat.com; yronglin...@gmail.com; Kumar N, 
Bhuvanendra ; 1135831...@qq.com; 
gandhi21...@gmail.com; tbae...@redhat.com; mlek...@skidmore.edu; 
blitzrak...@gmail.com; shen...@google.com
Subject: [PATCH] D141437: [HIP] Use .hipi as preprocessor output extension

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


thakis added a comment.

I think this breaks tests on Windows: 
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2F45.33.8.238%2Fwin%2F73191%2Fstep_7.txt&data=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JihDs%2FYNuK5FqlQnlpeH6HHfMuVykklmOg0MgUbJ4A8%3D&reserved=0

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD141437%2Fnew%2F&data=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=36LDFan8FQ%2BnroFc8LAaN5dJENQeMvb6eytkNaRHzps%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD141437&data=05%7C01%7Cyaxun.liu%40amd.com%7C13de06bdc38e49edd6f308daf4550754%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090943311573611%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=J4SCWar%2BKIF2eMoXtxHz1LSrgoxCU7NoPCBmQFxVg7U%3D&reserved=0

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

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

Will do. Thanks.

Sam

-Original Message-
From: Erich Keane via Phabricator  
Sent: Wednesday, February 19, 2020 8:52 AM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: erich.ke...@intel.com; echri...@gmail.com; jpien...@google.com; 
mask...@google.com; michael.hl...@gmail.com; mariya.podchishcha...@intel.com; 
alexey.ba...@intel.com; zhang.guans...@gmail.com; her...@google.com; 
r...@google.com; cfe-commits@lists.llvm.org; mlek...@skidmore.edu; 
blitzrak...@gmail.com; shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

erichkeane added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

Note that when recommitting this (if you choose to), this needs to also handle 
NamespaceDecl.  We're a downstream and discovered that this doesn't properly 
handle functions or records handled in a namespace.

It can be implemented identically to TranslationUnitDecl.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7C9e1ec947ff5e4e65a52408d7b55bfd7b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177279020210466&sdata=KdCHlCMkdeIhQ8%2B11lXTxz8srsvmYKtk2UiIR5aYheo%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172&data=02%7C01%7Cyaxun.liu%40amd.com%7C9e1ec947ff5e4e65a52408d7b55bfd7b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177279020210466&sdata=iZhUpTdtwu8kR%2FIRvhMVwMbig28F960X9LWzPTGO9WI%3D&reserved=0


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

RE: [PATCH] D76262: [NFC] Add UsedDeclVisitor

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

I am taking a look. Thanks.

Sam

-Original Message-
From: Jacques Pienaar via Phabricator  
Sent: Wednesday, March 18, 2020 6:14 PM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com
Cc: jpien...@google.com; cfe-commits@lists.llvm.org; mlek...@skidmore.edu; 
blitzrak...@gmail.com; shen...@google.com
Subject: [PATCH] D76262: [NFC] Add UsedDeclVisitor

[CAUTION: External Email]

jpienaar added a comment.

This does not appear to be NFC:
 git checkout 704cd4d5d0754904361823588f203369c309deca 

 ; ninja check-mlir passes  git checkout 
08ab8c9af4dd27cb306b449edc9a9c50ed11194a 

 ; ninja check-mlir fails with:

  0.  Program arguments: Compiles/build_clang/bin/clang++ -DBUILD_EXAMPLES 
-DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/mlir/lib/Transforms -Illvm-project/mlir/lib/Transforms -Iinclude 
-Illvm-project/llvm/include -Illvm-project/mlir/include -Itools/mlir/include 
-fPIC -fvisibility-inl
  ines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall 
-Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
-Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color 
-ffunction-sections -fdata-sections -O3 -fno-exceptions -fno-rtti -UNDEBUG 
-std=c++14 -MD -MT 
tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o
 -MF 
tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o.d
 -o 
tools/mlir/lib/Transforms/CMakeFiles/MLIRTransforms.dir/AffineDataCopyGeneration.cpp.o
 -c llvm-project/mlir/lib/Transforms/AffineDataCopyGeneration.cpp
  1.   parser at end of file
  2.  Per-file LLVM IR generation
  3.  
/usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:848:5:
 Generating code for declaration 'std::make_unique'
  
build_clang/bin/clang++(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55796bf8436a]
  
build_clang/bin/clang++(_ZN4llvm3sys17RunSignalHandlersEv+0x34)[0x55796bf82204]
  build_clang/bin/clang++(_ZN4llvm3sys15CleanupOnSignalEm+0xf8)[0x55796bf82708]
  build_clang/bin/clang++(+0x1a58a08)[0x55796bf07a08]
  /lib/x86_64-linux-gnu/libpthread.so.0(+0x13520)[0x7f1b727f7520]
  /lib/x86_64-linux-gnu/libc.so.6(abort+0x121)[0x7f1b722bd535]
  /lib/x86_64-linux-gnu/libc.so.6(+0x2540f)[0x7f1b722bd40f]
  /lib/x86_64-linux-gnu/libc.so.6(+0x32b92)[0x7f1b722cab92]
  
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction17EmitDeclRefLValueEPKNS_11DeclRefExprE+0x358)[0x55796c4cb138]
  
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction10EmitLValueEPKNS_4ExprE+0x2f9)[0x55796c4ca6d9]
  
build_clang/bin/clang++(_ZN5clang7CodeGen15CodeGenFunction14EmitCastLValueEPKNS_8CastExprE+0x103)[0x55796c4d4033]


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD76262%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7Cc0896d5c9d714310dc4808d7cb89a135%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637201664290055700&sdata=sioR1S3m471EXAI17oUWgqmOeX26gk81APYiWOINl4M%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD76262&data=02%7C01%7Cyaxun.liu%40amd.com%7Cc0896d5c9d714310dc4808d7cb89a135%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637201664290055700&sdata=bPmzDOZM%2FHIO9hyIP0YIV5NLhpjF3luLTC%2Bw6BQYLd8%3D&reserved=0


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

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

Thanks a lot for fixing it.

Sam

-Original Message-
From: Fangrui Song via Phabricator  
Sent: Sunday, February 16, 2020 8:41 PM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

MaskRay added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:14

+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTContext.h"

hliao wrote:
> this file is missing and breaks the build
Fixed by c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7Cd887ec3ab69146d8ee4e08d7b36392d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175112565124973&sdata=16dU6RUcV1M9%2BMoNbgytch179KaG6vr9fJO%2BzeYJ4z4%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172&data=02%7C01%7Cyaxun.liu%40amd.com%7Cd887ec3ab69146d8ee4e08d7b36392d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175112565124973&sdata=gDh1FqkmR7VzE73YXtGyX8Z3pr8FCJwjnZH9RaVv%2FCo%3D&reserved=0


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

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

I am taking a look. Thanks.

Sam

-Original Message-
From: Jacques Pienaar via Phabricator  
Sent: Monday, February 17, 2020 10:58 AM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: jpien...@google.com; mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

jpienaar added a comment.

This seems to result in triggering clang/lib/CodeGen/CGExpr.cpp:2626 when 
compiling mlir/lib/Transforms/AffineDataCopyGeneration.cpp with clang build 
with assertions on (clean build at e8e078c 

 just before this change, broken at this, assert triggering at build fix 
commit).

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbuildkite.com%2Fmlir%2Fmlir-core%2Fbuilds%2F2792%23a54fb239-718b-4f0b-a309-f83e46ceb252&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=nma5u9l5h6tkzQEX0hVfYe6VJQBXubt%2FCOF%2BWyWSujM%3D&reserved=0


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=yGGqSBdf6zVYW7IDFW57UEagQAyvaOfn2a8xu%2BaIr9o%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=sbHpgiR9tKyv0%2F4ZnuhUX%2BFaIHdtwoHTyOJxX1%2F46mg%3D&reserved=0


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

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

It seems to be caused by some commit hash before 
20c5968e0953d978be4d9d1062801e8758c393b5.

Sam

-Original Message-
From: Jacques Pienaar via Phabricator  
Sent: Monday, February 17, 2020 10:58 AM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: jpien...@google.com; mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

jpienaar added a comment.

This seems to result in triggering clang/lib/CodeGen/CGExpr.cpp:2626 when 
compiling mlir/lib/Transforms/AffineDataCopyGeneration.cpp with clang build 
with assertions on (clean build at e8e078c 

 just before this change, broken at this, assert triggering at build fix 
commit).

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbuildkite.com%2Fmlir%2Fmlir-core%2Fbuilds%2F2792%23a54fb239-718b-4f0b-a309-f83e46ceb252&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=nma5u9l5h6tkzQEX0hVfYe6VJQBXubt%2FCOF%2BWyWSujM%3D&reserved=0


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=yGGqSBdf6zVYW7IDFW57UEagQAyvaOfn2a8xu%2BaIr9o%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172&data=02%7C01%7Cyaxun.liu%40amd.com%7C929d8d11429a4c4072e108d7b3db6042%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175627129064872&sdata=sbHpgiR9tKyv0%2F4ZnuhUX%2BFaIHdtwoHTyOJxX1%2F46mg%3D&reserved=0


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

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

Probably I missed something. I will look again.

Thanks.

Sam

-Original Message-
From: Jacques Pienaar  
Sent: Tuesday, February 18, 2020 8:56 AM
To: Liu, Yaxun (Sam) 
Cc: reviews+d70172+public+e13d5528b180f...@reviews.llvm.org; t...@google.com; 
rjmcc...@gmail.com; rich...@metafoo.co.uk; jdoerf...@anl.gov; 
a.bat...@hotmail.com; mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: Re: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Liu, Yaxun (Sam) via cfe-commits]

[AMD Official Use Only - Internal Distribution Only]

Reverted.

I will make sure it does not regress mlir before commit again.

Thanks.

Sam

-Original Message-
From: Eric Christopher via Phabricator  
Sent: Tuesday, February 18, 2020 11:21 AM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: jpien...@google.com; mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

echristo added a comment.

In D70172#1879481 
,
 @jpienaar wrote:

> This seems to result in triggering clang/lib/CodeGen/CGExpr.cpp:2626 when 
> compiling mlir/lib/Transforms/AffineDataCopyGeneration.cpp with clang build 
> with assertions on (clean build at e8e078c 
> 
>  just before this change, broken at this, assert triggering at build fix 
> commit).
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbuildkite.com%2Fmlir%2Fmlir-core%2Fbuilds%2F2792%23a54fb239-718b-4f0b-a309-f83e46ceb252&data=02%7C01%7Cyaxun.liu%40amd.com%7Ce901b932c7c140a8a22c08d7b4a7b390%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637176504711389865&sdata=8OqEGO2vIUaVIEsaRfhzmTgJRGnMFR2ZpXH2LSkr9Ds%3D&reserved=0


Seems reasonable to revert if there's a testcase that they can get from 
rebuilding llvm with mlir enabled.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2F&data=02%7C01%7Cyaxun.liu%40amd.com%7Ce901b932c7c140a8a22c08d7b4a7b390%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637176504711389865&sdata=64lMij5bibrJnOLdYufLxO5u3GNpJe%2BzoSOdYef7zyw%3D&reserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172&data=02%7C01%7Cyaxun.liu%40amd.com%7Ce901b932c7c140a8a22c08d7b4a7b390%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637176504711389865&sdata=hfZYdMzByxpdoxtqLEP%2FAHWNKBHWkcXRbqXtrSqCfE4%3D&reserved=0


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

RE: [PATCH] D44747: Set calling convention for CUDA kernel

[Liu, Yaxun (Sam) via cfe-commits]

I will try fixing that.

The CUDA kernel calling convention should be dropped in all DRE's since it is 
invisible to the user.

Sam

-Original Message-
From: Artem Belevich via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, April 03, 2018 1:51 PM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Arsenault, 
Matthew 
Cc: llvm-comm...@lists.llvm.org; t...@google.com; Zhuravlyov, Konstantin 
; wei.di...@amd.com; Stuttard, David 
; tpr.l...@botech.co.uk; Tye, Tony ; 
cfe-commits@lists.llvm.org; jle...@google.com
Subject: [PATCH] D44747: Set calling convention for CUDA kernel

tra added inline comments.



Comment at: lib/Sema/SemaType.cpp:3319-3330
+  // Attribute AT_CUDAGlobal affects the calling convention for AMDGPU targets.
+  // This is the simplest place to infer calling convention for CUDA kernels.
+  if (S.getLangOpts().CUDA && S.getLangOpts().CUDAIsDevice) {
+for (const AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
+ Attr; Attr = Attr->getNext()) {
+  if (Attr->getKind() == AttributeList::AT_CUDAGlobal) {
+CC = CC_CUDAKernel;

tra wrote:
> This apparently breaks compilation of some CUDA code in our internal tests. 
> I'm working on minimizing a reproduction case. Should this code be enabled 
> for AMD GPUs only?
Here's a small snippet of code that previously used to compile and work:

```
template 
__global__ void EmptyKernel(void) { }

struct Dummy {
  /// Type definition of the EmptyKernel kernel entry point
  typedef void (*EmptyKernelPtr)();
  EmptyKernelPtr Empty() { return EmptyKernel; } }; ``` AFAICT,  it's 
currently impossible to apply __global__ to pointers, so there's no way to make 
the code above work with this patch applied.


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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

RE: [PATCH] D44747: Set calling convention for CUDA kernel

[Liu, Yaxun (Sam) via cfe-commits]

Let's revert it for now. I will create a review after fixing it and commit it 
again with the fix.

Thanks.

Sam 

-Original Message-
From: Artem Belevich via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, April 03, 2018 2:09 PM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Arsenault, 
Matthew 
Cc: jle...@google.com; llvm-comm...@lists.llvm.org; t...@google.com; 
Zhuravlyov, Konstantin ; wei.di...@amd.com; 
Stuttard, David ; tpr.l...@botech.co.uk; Tye, Tony 
; cfe-commits@lists.llvm.org
Subject: [PATCH] D44747: Set calling convention for CUDA kernel

tra added a comment.

In https://reviews.llvm.org/D44747#1055877, @yaxunl wrote:

> I will try fixing that.
>
> The CUDA kernel calling convention should be dropped in all DRE's since it is 
> invisible to the user.
>
> Sam


Apparently it's not always the case.
Here's a bit more elaborate example demonstrating inconsistency in this 
behavior. Calling convention is ignored for regular functions, but not for 
function templates.

  __global__ void EmptyKernel(void) { }
  
  template 
  __global__ void EmptyKernelT(void) { }
  
  struct Dummy {
/// Type definition of the EmptyKernel kernel entry point
typedef void (*EmptyKernelPtr)();
EmptyKernelPtr Empty() { return EmptyKernel; } // this one works
EmptyKernelPtr EmptyT() { return EmptyKernelT; } // this one errors 
out.
  };

Do you think this is something you can fix quickly or do you want to unroll the 
change while you figure out what's going on?


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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

RE: r289991 - Revert r289979 due to regressions

[Liu, Yaxun (Sam) via cfe-commits]

Thanks.

Sam

From: Reid Kleckner [mailto:r...@google.com]
Sent: Friday, December 16, 2016 5:24 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits 
Subject: Re: r289991 - Revert r289979 due to regressions

This revert broke the build because you failed to resolve conflicts in 
include/clang/Basic/OpenCLOptions.h caused by r289985. I've reverted that file 
in r289997.

On Fri, Dec 16, 2016 at 1:23 PM, Yaxun Liu via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: yaxunl
Date: Fri Dec 16 15:23:55 2016
New Revision: 289991
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r298767 - [AMDGPU] Switch address space mapping by triple environment amdgiz

[Liu, Yaxun (Sam) via cfe-commits]

Thanks for your comments. I will fix that.

Sam

From: Eric Christopher [mailto:echri...@gmail.com]
Sent: Saturday, March 25, 2017 1:52 AM
To: Liu, Yaxun (Sam) ; cfe-commits@lists.llvm.org
Subject: Re: r298767 - [AMDGPU] Switch address space mapping by triple 
environment amdgiz


On Fri, Mar 24, 2017 at 8:58 PM Yaxun Liu via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: yaxunl
Date: Fri Mar 24 22:46:25 2017
New Revision: 298767

URL: http://llvm.org/viewvc/llvm-project?rev=298767&view=rev
Log:
[AMDGPU] Switch address space mapping by triple environment amdgiz

For target environment amdgiz and amdgizcl (giz means Generic Is Zero), AMDGPU 
will use new address space mapping where generic address space is 0 and private 
address space is 5. The data layout is also changed correspondingly.

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

Added:

cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
Modified:
cfe/trunk/lib/Basic/Targets.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=298767&r1=298766&r2=298767&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Fri Mar 24 22:46:25 2017
@@ -2027,14 +2027,23 @@ ArrayRef NVPTXTargetInfo::
   return llvm::makeArrayRef(GCCRegNames);
 }

-static const unsigned AMDGPUAddrSpaceMap[] = {
-  1,// opencl_global
-  3,// opencl_local
-  2,// opencl_constant
-  4,// opencl_generic
-  1,// cuda_device
-  2,// cuda_constant
-  3 // cuda_shared
+static const LangAS::Map AMDGPUPrivateIsZeroMap = {
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+4,  // opencl_generic
+1,  // cuda_device
+2,  // cuda_constant
+3   // cuda_shared
+};
+static const LangAS::Map AMDGPUGenericIsZeroMap = {
+1,  // opencl_global
+3,  // opencl_local
+4,  // opencl_constant
+0,  // opencl_generic
+1,  // cuda_device
+4,  // cuda_constant
+3   // cuda_shared
 };

 // If you edit the description strings, make sure you update
@@ -2044,15 +2053,39 @@ static const char *const DataLayoutStrin
   "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
   "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";

-static const char *const DataLayoutStringSI =
+static const char *const DataLayoutStringSIPrivateIsZero =
   "e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32"
   "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
   "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";

+static const char *const DataLayoutStringSIGenericIsZero =
+  "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32"
+  "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
+  "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";
+
 class AMDGPUTargetInfo final : public TargetInfo {
   static const Builtin::Info BuiltinInfo[];
   static const char * const GCCRegNames[];

+  struct AddrSpace {
+unsigned Generic, Global, Local, Constant, Private;
+AddrSpace(bool IsGenericZero_ = false){
+  if (IsGenericZero_) {
+Generic   = 0;
+Global= 1;
+Local = 3;
+Constant  = 4;
+Private   = 5;
+  } else {
+Generic   = 4;
+Global= 1;
+Local = 3;
+Constant  = 2;
+Private   = 0;
+  }
+}
+  };
+
   /// \brief The GPU profiles supported by the AMDGPU target.
   enum GPUKind {
 GK_NONE,
@@ -2079,6 +2112,10 @@ class AMDGPUTargetInfo final : public Ta
 return TT.getArch() == llvm::Triple::amdgcn;
   }

+  static bool isGenericZero(const llvm::Triple &TT) {
+return TT.getEnvironmentName() == "amdgiz" ||
+TT.getEnvironmentName() == "amdgizcl";
+  }
 public:
   AMDGPUTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
 : TargetInfo(Triple) ,
@@ -2086,17 +2123,21 @@ public:
   hasFP64(false),
   hasFMAF(false),
   hasLDEXPF(false),
-  hasFullSpeedFP32Denorms(false){
+  hasFullSpeedFP32Denorms(false),
+  AS(isGenericZero(Triple)){
 if (getTriple().getArch() == llvm::Triple::amdgcn) {
   hasFP64 = true;
   hasFMAF = true;
   hasLDEXPF = true;
 }
-
+auto IsGenericZero = isGenericZero(Triple);
 resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn ?
-DataLayoutStringSI : DataLayoutStringR600);
+(IsGenericZero ? DataLayoutStringSIGenericIsZero :
+DataLayoutStringSIPrivateIsZero)
+: DataLayoutStringR600);

-AddrSpaceMap = &AMDGPUAddrSpaceMap;
+AddrSpaceMap = IsGenericZero ? &AMDGPUGenericIsZeroMap :
+&AMDGPUPrivateIsZeroMap;
 UseAddrSpaceMapMangling = true;
   }

@@ -2104,14 +2145,10 @@ public:
 if (GPU <= GK_CAYMAN)
   return 32;

-switch(AddrSpace) {
-  default:
-  

RE: PATCH: re-enable OpenCL extensions

[Liu, Yaxun (Sam) via cfe-commits]

I think the supported extensions for a target should be as accurate as 
possible, for it to be useful. Setting all extensions to be supported on all 
targets will defeat its purpose.

I recommend to introduce "pocl" as an environment in the triple and add 
supported OpenCL extensions for different targets based on that.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Thursday, January 19, 2017 12:31 PM
To: Kalle Raiskila 
Cc: cfe-commits@lists.llvm.org; Liu, Yaxun (Sam) ; nd 

Subject: RE: PATCH: re-enable OpenCL extensions

As mentioned on cfe-dev as well, although it doesn't seem too critical it is 
generally not logical to enable all extensions by default because most of the 
targets don't even support OpenCL. But I understand your situation with using 
x86 or ARM backends in a generic way. Do you think this can be solved instead 
with the new " -cl-ext=" option: 
http://clang.llvm.org/docs/UsersManual.html#opencl-specific-options

May be Sam could comment more since he has done most work with the extensions 
lately.

Cheers,
Anastasia

-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Kalle Raiskila via cfe-commits
Sent: 19 January 2017 08:03
To: cfe-commits@lists.llvm.org
Subject: PATCH: re-enable OpenCL extensions

Hi,

I noticed a change from clang 3.8 to 3.9, that it disabled all OpenCL extension 
pragmas per default.
This broke pocl on e.g. ARM for LLVM 3.9 
(https://github.com/pocl/pocl/issues/409).

Example:
$ echo "#pragma OPENCL EXTENSION cl_khr_icd: enable" > hello.cl $ clang  
-emit-llvm -x cl -o tmp.bc -c hello.cl

works fine, but:
$ clang  -emit-llvm -x cl -o tmp.bc -c hello.cl 
--target=armv7-unknown-linux-gnueabihf
hello.cl:1:26: warning: unsupported OpenCL extension 'cl_khr_icd' - ignoring 
[-Wignored-pragmas] #pragma OPENCL EXTENSION cl_khr_icd: enable
   ^
1 warning generated.


Attached is a patch that enables OpenCL extensions for all targets per default, 
and then sets the status quo of supported extensions for those targets that 
currently customize their settings (i.e.
NVPTX and AMDGPU).
Most generic CPUs can handle all OpenCL extensions just fine.

Please keep me as CC, I am not subscribed to the list.
thanks,
kalle

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

RE: D32248: CodeGen: Cast alloca to expected address space

[Liu, Yaxun (Sam) via cfe-commits]

I think if there is no triple environment or if the triple environment is 
"opencl", I should always map default address space to 0. Then it should not 
break libclc and previous OpenCL applications.

Sam

-Original Message-
From: Tom Stellard via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, May 23, 2017 9:52 AM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Tye, Tony 
; anastasia.stul...@arm.com
Cc: tstel...@redhat.com; nhaeh...@gmail.com; Chan, SiuChi 
; whch...@gmail.com; Sumner, Brian ; 
cfe-commits@lists.llvm.org
Subject: [PATCH] D32248: CodeGen: Cast alloca to expected address space

tstellar added inline comments.



Comment at: cfe/trunk/lib/Basic/Targets.cpp:2037-2038
 
-static const LangAS::Map AMDGPUPrivateIsZeroMap = {
-4,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-4,  // opencl_generic
-1,  // cuda_device
-2,  // cuda_constant
-3   // cuda_shared
-};
-static const LangAS::Map AMDGPUGenericIsZeroMap = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_generic
-1,  // cuda_device
-2,  // cuda_constant
-3   // cuda_shared
+static const LangAS::Map AMDGPUNonOpenCLPrivateIsZeroMap = {
+4, // Default
+1, // opencl_global

Shouldn't this be 0 ?


Repository:
  rL LLVM

https://reviews.llvm.org/D32248



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

RE: [PATCH] D25343: [OpenCL] Mark group functions as convergent in opencl-c.h

[Liu, Yaxun (Sam) via cfe-commits]

+ Tom Matt

Thanks Ettore.
 
I think OpenCL is subject to the same issue, and noduplicate does not help 
either.

Basically if a function A directly or indirectly calls a convergent function 
e.g. barrier, function A itself must also be marked as convergent, otherwise 
optimization passes may transform a convergent call of A into multiple 
divergent calls of A.

That means if we only know the declaration of a function, we have to assume it 
is convergent since in its body it may call a convergent function.

I think probably OpenCL should take the same approach, i.e., mark all functions 
as convergent, then let Transforms/IPO/FunctionAttrs.cpp to remove unnecessary 
convergent attribute.

Sam

-Original Message-
From: Ettore Speziale [mailto:speziale.ett...@gmail.com] 
Sent: Thursday, October 20, 2016 11:42 AM
To: reviews+d25343+public+a10e9553b0fc8...@reviews.llvm.org; Liu, Yaxun (Sam) 

Cc: Ettore Speziale ; alexey.ba...@intel.com; 
anastasia.stul...@arm.com; aaron.ball...@gmail.com; Clang Commits 
; Sumner, Brian 
Subject: Re: [PATCH] D25343: [OpenCL] Mark group functions as convergent in 
opencl-c.h

Hello guys,

>> Should we deprecate noduplicate then as convergent should cover both use 
>> cases for OpenCL I believe? As far as I understand noduplicate was added 
>> specifically for SPMD use cases...
> 
> noduplicate has different semantics than convergent. Although it is proposed 
> for SPMD originally, it could be used by non-SPMD programs to forbid 
> duplicate of functions. There may be applications using this attribute.
> 
> I would suggest to leave this question for future. Probably ask llvm-dev 
> first since the attribute is also in LLVM.

I just want to clarify why I withdraw the convergent patch I initially 
submitted some time ago. It has a problem when dealing with multiple modules. 
Consider the following example:

int foo(void) __attribute__((pure));

int bar(int x) {
  int y = foo();
  if (x)
return y;
  return 0;
}   

I’ve just marked foo with the pure attribute to mark the function readonly in 
LLVM IR. Given that IR, the IR sinking pass pushes the foo call site into the 
then branch:

; Function Attrs: nounwind readonly ssp uwtable define i32 @bar(i32) #0 {
  %2 = icmp eq i32 %0, 0
  br i1 %2, label %5, label %3

; :3   ; preds = %1
  %4 = tail call i32 @foo() #2
  br label %5

; :5   ; preds = %1, %3
  %6 = phi i32 [ %4, %3 ], [ 0, %1 ]
  ret i32 %6
}

; Function Attrs: nounwind readonly
declare i32 @foo() #1

This is kind of dangerous, as we do not know what is inside foo — i.e. it might 
contains a convergent call.

If I understand correctly, the CUDA guys solved the problem in two steps. At 
CodeGen time all the device function calls are marked convergent:

  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
// Conservatively, mark all functions and calls in CUDA as convergent
// (meaning, they may call an intrinsically convergent op, such as
// __syncthreads(), and so can't have certain optimizations applied around
// them).  LLVM will remove this attribute where it safely can.
FuncAttrs.addAttribute(llvm::Attribute::Convergent);

Then LLVM function attribute pass — lib/Transforms/IPO/FunctionAttrs.cpp — 
remove the unnecessary convergent attributes starting from the leaf nodes  — 
i.e. external calls.

Provide that intrinsics are correctly marked convergent only when needed, that 
allow to get rid of the unnecessary convergent attributes.

Since you are introducing an explicit convergent attribute it seems that OpenCL 
is requiring the developers to explicitly mark the functions that might contain 
convergent function calls with the convergent attribute. Am I understand 
correctly?

Thanks

--
Ettore Speziale — Compiler Engineer
speziale.ett...@gmail.com
espezi...@apple.com
--

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

RE: [PATCH] D25343: [OpenCL] Mark group functions as convergent in opencl-c.h

[Liu, Yaxun (Sam) via cfe-commits]

Just my two cents.

Let's consider a simple example:

for (int I = 0; I < 2; I++) {
  A = Compute();
  barrier();
  Use(A);
}

This does not mean Compute() needs to be executed for all the loop iterations 
before barrier() being executed. This only means that during each loop 
iteration, Compute() needs to be executed by all threads before barrier is 
executed by all threads. Otherwise, we cannot use barrier() in a loop at all.

Semantically the above program is not different from the following one:

  A = Compute();
  barrier();
  Use(A);
  A = Compute();
  barrier();
  Use(A);

Even if Compute() and Use() have side effect, they are still equivalent 
programs.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Monday, October 24, 2016 1:08 PM
To: Liu, Yaxun (Sam) ; alexey.ba...@intel.com; 
anastasia.stul...@arm.com; aaron.ball...@gmail.com
Cc: Stellard, Thomas ; Arsenault, Matthew 
; Sumner, Brian ; 
cfe-commits@lists.llvm.org
Subject: [PATCH] D25343: [OpenCL] Mark group functions as convergent in 
opencl-c.h

Anastasia added a comment.

In https://reviews.llvm.org/D25343#567374, @tstellarAMD wrote:

> In https://reviews.llvm.org/D25343#565288, @Anastasia wrote:
>
> > Do you have any code example where Clang/LLVM performs wrong optimizations 
> > with respect to the control flow of SPMD execution?
> >
> > My understanding from the earlier discussion we have had: 
> > https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg22643.html that 
> > noduplicate is essentially enough for the frontend to prevent erroneous 
> > optimizations. Because in general compiler can't do much with unknown 
> > function calls.
>
>
> noduplicate is enough for correctness, but it prevents legal optimizations, 
> like unrolling loops with barriers.  The convergent attribute was added 
> specifically for these kinds of builtins, so we should be using it here 
> instead of noduplicate.
>
> > For LLVM intrinsics it is slightly different as I can deduce from this 
> > discussion:http://lists.llvm.org/pipermail/llvm-dev/2015-May/085558.html . 
> > It seems like by default it's assumed to be side effect free and can be 
> > optimized in various ways.




In https://reviews.llvm.org/D25343#567374, @tstellarAMD wrote:

> In https://reviews.llvm.org/D25343#565288, @Anastasia wrote:
>
> > Do you have any code example where Clang/LLVM performs wrong optimizations 
> > with respect to the control flow of SPMD execution?
> >
> > My understanding from the earlier discussion we have had: 
> > https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg22643.html that 
> > noduplicate is essentially enough for the frontend to prevent erroneous 
> > optimizations. Because in general compiler can't do much with unknown 
> > function calls.
>
>
> noduplicate is enough for correctness, but it prevents legal optimizations, 
> like unrolling loops with barriers.  The convergent attribute was added 
> specifically for these kinds of builtins, so we should be using it here 
> instead of noduplicate.
>
> > For LLVM intrinsics it is slightly different as I can deduce from this 
> > discussion:http://lists.llvm.org/pipermail/llvm-dev/2015-May/085558.html . 
> > It seems like by default it's assumed to be side effect free and can be 
> > optimized in various ways.


Tom, as far as I understand a valid generalized use case with a barrier is as 
follows:

  a = compute() 
  barrier()
  use(a)

Regarding unrolling I don't see how the barrier can be unrolled without 
breaking the correctness of OpenCL programs because all compute() calls of one 
WG have to complete before use(a) is invoked for the first time. I am not an 
expert in LLVM transformations but if I read the description of both 
noduplicate and convergent (http://llvm.org/docs/LangRef.html) none of those 
seem to make sense to me for the barrier because both allow reordering within 
the same function or BB respectively and the only valid way seems to mark it as 
a side-effect intrinsic to prevent any reordering of the barrier itself. I 
understand within compute() or use() independently there are not limitations on 
ordering of instructions.

If you have any specific example in mind where the unrolling would work could 
you please elaborate on it. I think it's essential for the community to 
undertsnad the use cases for noduplicate/convergent/sideeffect to make better 
use of them also across similar models i.e. OpenCL, CUDA, etc.


https://reviews.llvm.org/D25343



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

RE: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

[Liu, Yaxun (Sam) via cfe-commits]

Hi Richard/Anastasia,

I agree some tests are redundant. I will try to remove them.

Thanks.

Sam

From: Anastasia Stulova [mailto:anastasia.stul...@arm.com]
Sent: Friday, November 04, 2016 7:43 AM
To: Richard Smith ; Liu, Yaxun (Sam) 
Cc: nd ; cfe-commits@lists.llvm.org
Subject: RE: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

Hi Richard,

I guess the problem is that the header file itself is really large and we have 
all these configurations that depending on versions/extensions expose different 
builtin libraries. They are all based on the core part though which is already 
large.

Ideally it would be nice to test the full functionality but I agree the price 
is probably quite high at the moment. Something we could immediately cut down 
though is all configurations with –fblocks. I think we are implicitly setting 
it now with –std=CL2.0.

Also I am not sure why we are testing different targets. I don’t think we have 
many target specific bits in the header? Apart from the common code I can I 
only see AMD specific part. Perhaps testing for SPIR and AMD targets only 
should be enough? Also the tests doesn’t really check for anything AMD 
specific. Perhaps that could be improved as well…

Cheers,
Anastasia

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Richard Smith via cfe-commits
Sent: 03 November 2016 22:10
To: Yaxun Liu
Cc: cfe-commits
Subject: Re: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

Hi,

This test (test/Headers/opencl-c-header.cl) is 
*extremely* slow -- it parses the 17KLoC opencl-c.h header 40 times, and on my 
Debug builds it's the slowest test by far, taking nearly 2 minutes to run 
*alone*. (For reference, the *entire clang test suite* finishes in 90s on my 
machine with this test removed.)

Please can you do something about this? Do we really need to parse this header 
in 40 configurations here?

On Mon, Jun 20, 2016 at 12:26 PM, Yaxun Liu via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: yaxunl
Date: Mon Jun 20 14:26:00 2016
New Revision: 273191

URL: http://llvm.org/viewvc/llvm-project?rev=273191&view=rev
Log:
[OpenCL] Include opencl-c.h by default as a clang module

Include opencl-c.h by default as a module to utilize the automatic AST caching 
mechanism of clang modules.

Add an option -finclude-default-header to enable default header for OpenCL, 
which is off by default.

Differential Revision: http://reviews.llvm.org/D20444

Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Headers/module.modulemap
cfe/trunk/test/Headers/opencl-c-header.cl

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=273191&r1=273190&r2=273191&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Mon Jun 20 14:26:00 2016
@@ -218,7 +218,7 @@ LANGOPT(ObjCWeak, 1, 0, "Obj
 LANGOPT(ObjCSubscriptingLegacyRuntime , 1, 0, "Subscripting support in 
legacy ObjectiveC runtime")
 LANGOPT(FakeAddressSpaceMap , 1, 0, "OpenCL fake address space map")
 ENUM_LANGOPT(AddressSpaceMapMangling , AddrSpaceMapMangling, 2, ASMM_Target, 
"OpenCL address space map mangling mode")
-
+LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
 BENIGN_LANGOPT(DelayedTemplateParsing , 1, 0, "delayed template parsing")
 LANGOPT(BlocksRuntimeOptional , 1, 0, "optional blocks runtime")


Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=273191&r1=273190&r2=273191&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Jun 20 14:26:00 2016
@@ -612,6 +612,8 @@ def fallow_half_arguments_and_returns :
   HelpText<"Allow function arguments and returns of type half">;
 def fdefault_calling_conv_EQ : Joined<["-"], "fdefault-calling-conv=">,
   HelpText<"Set default MS calling convention">;
+def finclude_default_header : Flag<["-"], "finclude-default-header">,
+  HelpText<"Include the default header file for OpenCL">;

 // C++ TSes.
 def fcoroutines : Flag<["-"], "fcoroutines">,

Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=273191&r1=273190&r2=273191&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerIn

RE: D26157: [OpenCL] always use SPIR address spaces for kernel_arg_addr_space MD

[Liu, Yaxun (Sam) via cfe-commits]

Even if we use string for address space MD, we still need to define them 
somewhere. On the other hand, if we use SPIR enum, we just need to refer to the 
SPIR spec in the documenting comments.

Sam

-Original Message-
From: Pekka Jääskeläinen [mailto:pekka.jaaskelai...@gmail.com] 
Sent: Thursday, November 03, 2016 4:24 PM
To: pekka.jaaskelai...@gmail.com; Stellard, Thomas ; 
anastasia.stul...@arm.com; Liu, Yaxun (Sam) 
Cc: alexey.ba...@intel.com; xiuli...@outlook.com; cfe-commits@lists.llvm.org; 
Ding, Wei 
Subject: [PATCH] D26157: [OpenCL] always use SPIR address spaces for 
kernel_arg_addr_space MD

pekka.jaaskelainen added a comment.

Indeed, it requires wider scale discussion to get it right, and e.g. to pass 
the info to AA. But to be honest, I think OpenCL and CUDA are still considered 
'minority' languages in Clang/LLVM which makes me usually lean towards least 
intrusive implementation solutions whenever possible.

The matter was discussed 5 years ago: 
http://clang-developers.42468.n3.nabble.com/OpenCL-Address-Spaces-and-Runtimes-td2814865.html

The current situation of having the target AS in the MD is clearly not the way 
to go due to the loss of info for flat AS machines, so I think this discussion 
should be mostly about what should those designated IDs be.

Sure, we could use strings such as "opencl.global" there instead, but I'm not 
sure how much that adds value to simply having known integer IDs instead. SPIR 
1.2-2.0 is based on LLVM and designed to store info of OpenCL C kernels, 
therefore I thought the SPIR IDs are logical as we can just refer to SPIR specs 
in this case. Other alternative might be to retain the Clang's logical IDs, but 
I recall there was some special semantics/handling for AS IDs > 255. I might be 
wrong though.


Repository:
  rL LLVM

https://reviews.llvm.org/D26157



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

RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

[Liu, Yaxun (Sam) via cfe-commits]

The LLVM IR generated by Clang trunk for spir target is not conformant to SPIR 
spec 1.2 or 2.0 even without my change. For example, it differs from SPIR spec 
about LLVM version, image type name, sampler type representation, blocks 
representation, etc. It is the result of evolution of the original SPIR for a 
better solution of many issues of SPIR.

If we look back at the RFC about the new kernel argument metadata, there was 
agreement that the original kernel argument metadata is difficult to work with, 
therefore the proposal to use function metadata was accepted to alleviate this 
issue. The new function metadata was more concise and easier to work with.

For SPIR consumer, the new format conveys equivalent information as the old 
format. There is no reason they cannot consume this new format.

If users want to generate SPIR conformant LLVM IR, they should use khronos SPIR 
producer instead of clang trunk.

Sam

-Original Message-
From: Pan Xiuli [mailto:xiuli...@outlook.com] 
Sent: Monday, August 1, 2016 10:40 PM
To: 'Anastasia Stulova' ; 
reviews+d20979+public+e2872c9c869f1...@reviews.llvm.org; 
alexey.ba...@intel.com; Liu, Yaxun (Sam) 
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org; 'nd' 

Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel 
attributes

Hi,

What I mean is the spec of spir 1.2/2.0 has very clear example about how the 
metadata should organized. But now with this patch, the result of spir is 
changed and not like it before. So what I want to know is why we change the old 
spir example style code, but not add something alongside.
Now we have metadata after each function but spir shows a opencl.kernels 
metadata is need for all modules:
"Each SPIR module has a opencl.kernels named metadata node containing a list of 
metadata objects."

I think this patch maybe useful for some other target, but we should not break 
the old spir as now we have not had a substitute one.

Thanks
Xiuli

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Tuesday, August 2, 2016 2:14 AM
To: Pan Xiuli ; 
reviews+d20979+public+e2872c9c869f1...@reviews.llvm.org; 
alexey.ba...@intel.com; Liu, Yaxun (Sam) (yaxun@amd.com) 
Cc: thomas.stell...@amd.com; cfe-commits@lists.llvm.org; nd 
Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel 
attributes

Hi Xiuli,

Could you please elaborate what do you think it broken exactly?

Because we haven't actually removed opencl.kernels metadata but just changed 
the format of it. 

Basically, we are using function metadata instead of generic metadata as it was 
before.

Thanks,
Anastasia

-Original Message-
From: Pan Xiuli [mailto:xiuli...@outlook.com] 
Sent: 01 August 2016 05:54
To: reviews+d20979+public+e2872c9c869f1...@reviews.llvm.org; Anastasia Stulova; 
alexey.ba...@intel.com
Cc: thomas.stell...@amd.com; cfe-commits@lists.llvm.org
Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel 
attributes

Hi Sam,

I am now using llvm39 to enable our opencl driver, but I found this patch broke 
the spir target as well.
The opencl.kernels metadata is required by spir spec. I think we should keep 
the old code for spir target.

Thanks
Xiuli

-Original Message-
From: Yaxun Liu [mailto:yaxun@amd.com] 
Sent: Wednesday, June 22, 2016 11:04 PM
To: yaxun@amd.com; xiuli...@outlook.com; anastasia.stul...@arm.com; 
alexey.ba...@intel.com
Cc: thomas.stell...@amd.com; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel 
attributes

This revision was automatically updated to reflect the committed changes.
Closed by commit rL273425: [OpenCL] Use function metadata to represent kernel 
attributes (authored by yaxunl).

Changed prior to commit:
  http://reviews.llvm.org/D20979?vs=60545&id=61555#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20979

Files:
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl
  cfe/trunk/test/CodeGenOpenCL/kernel-attributes.cl
  cfe/trunk/test/CodeGenOpenCL/kernel-metadata.cl

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

RE: [PATCH] D19484: [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Done. Thanks for letting me know.

Sam

-Original Message-
From: Jun Bum Lim [mailto:junb...@codeaurora.org] 
Sent: Friday, May 13, 2016 1:10 PM
To: Liu, Yaxun (Sam) ; anastasia.stul...@arm.com; 
aleksey.ba...@mail.ru
Cc: junb...@codeaurora.org; Stellard, Thomas ; 
xiuli...@outlook.com; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D19484: [OpenCL] Add supported OpenCL extensions to target 
info.

junbuml added a subscriber: junbuml.
junbuml added a comment.

In current trunk, I got build error :

  llvm/tools/clang/lib/Basic/Targets.cpp:2090:9: error: 
'setSupportedOpenCLOpts' overrides a member function but is not marked 
'override' [-Werror,-Winconsistent-missing-override]
   void setSupportedOpenCLOpts() {

Can you revert this change if this error is related with this change ?


Repository:
  rL LLVM

http://reviews.llvm.org/D19484



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

RE: r269431 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Thanks Steven.

I have reverted my change. I will apply your patch and re-commit.

Sam

-Original Message-
From: steve...@apple.com [mailto:steve...@apple.com] 
Sent: Friday, May 13, 2016 1:27 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits@lists.llvm.org
Subject: Re: r269431 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Yaxun

You seems missing some override keyword that triggers 
-Winconsistent-missing-override. See:
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_build/24442/warnings8Result/new/

Here is a patch to fix them:

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index 
b1b12e4..20f1e95 100644
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -2087,7 +2087,7 @@ public:
 return true;
   }
 
-   void setSupportedOpenCLOpts() {
+   void setSupportedOpenCLOpts() override {
  auto &Opts = getSupportedOpenCLOpts();
  Opts.cl_clang_storage_class_specifiers = 1;
  Opts.cl_khr_gl_sharing = 1;
@@ -2731,7 +2731,7 @@ public:
 return true;
   }
 
-  void setSupportedOpenCLOpts() {
+  void setSupportedOpenCLOpts() override {
 getSupportedOpenCLOpts().setAll();
   }
 };
@@ -7877,7 +7877,7 @@ public:
 return CC_SpirFunction;
   }
 
-  void setSupportedOpenCLOpts() {
+  void setSupportedOpenCLOpts() override {
 // Assume all OpenCL extensions and optional core features are supported
 // for SPIR since it is a generic target.
 getSupportedOpenCLOpts().setAll();

Thanks

Steven

> On May 13, 2016, at 8:44 AM, Yaxun Liu via cfe-commits 
>  wrote:
> 
> Author: yaxunl
> Date: Fri May 13 10:44:37 2016
> New Revision: 269431
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=269431&view=rev
> Log:
> [OpenCL] Add supported OpenCL extensions to target info.
> 
> Add supported OpenCL extensions to target info. It serves as default values 
> to save the users of the burden setting each supported extensions and 
> optional core features in command line.
> 
> Differential Revision: http://reviews.llvm.org/D19484
> 
> Added:
>cfe/trunk/include/clang/Basic/OpenCLOptions.h
>cfe/trunk/test/SemaOpenCL/extensions.cl
> Removed:
>cfe/trunk/test/SemaOpenCL/extension-fp64-cl1.1.cl
>cfe/trunk/test/SemaOpenCL/extension-fp64.cl
>cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl1.2.cl
>cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl2.0.cl
> Modified:
>cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>cfe/trunk/include/clang/Basic/LangOptions.h
>cfe/trunk/include/clang/Basic/OpenCLExtensions.def
>cfe/trunk/include/clang/Basic/TargetInfo.h
>cfe/trunk/include/clang/Basic/TargetOptions.h
>cfe/trunk/lib/Basic/Targets.cpp
>cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>cfe/trunk/lib/Parse/ParsePragma.cpp
>cfe/trunk/lib/Sema/Sema.cpp
>cfe/trunk/test/CodeGenOpenCL/builtins-r600.cl
>cfe/trunk/test/CodeGenOpenCL/fpmath.cl
>cfe/trunk/test/CodeGenOpenCL/half.cl
>cfe/trunk/test/Lexer/opencl-half-literal.cl
>cfe/trunk/test/Misc/languageOptsOpenCL.cl
>cfe/trunk/test/PCH/opencl-extensions.cl
>cfe/trunk/test/Parser/opencl-astype.cl
>cfe/trunk/test/Parser/opencl-atomics-cl20.cl
>cfe/trunk/test/Parser/opencl-pragma.cl
>cfe/trunk/test/Parser/opencl-storage-class.cl
>cfe/trunk/test/SemaOpenCL/half.cl
>cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl
>cfe/trunk/test/SemaOpenCL/invalid-logical-ops-1.2.cl
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diag
> nosticParseKinds.td?rev=269431&r1=269430&r2=269431&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri May 13 
> +++ 10:44:37 2016
> @@ -926,6 +926,10 @@ def warn_pragma_expected_enable_disable
>   "expected 'enable' or 'disable' - ignoring">, 
> InGroup; def warn_pragma_unknown_extension : Warning<
>   "unknown OpenCL extension %0 - ignoring">, InGroup;
> +def warn_pragma_unsupported_extension : Warning<
> +  "unsupported OpenCL extension %0 - ignoring">, 
> +InGroup; def warn_pragma_extension_is_core : Warning<
> +  "OpenCL extension %0 is core feature or supported optional core 
> +feature - ignoring">, InGroup;
> 
> // OpenCL errors.
> def err_opencl_taking_function_address_parser : Error<
> 
> Modified: cfe/trunk/include/clang/Basic/LangOptions.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Lang
> Options.h?rev=269431&r1=269430&r2=269431&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/LangOptions.h (original)
> +++ cfe/trunk/include/clang/Basic/LangOptions.h Fri May 13 10:44:37 
> +++ 2016
> @@ -160,18 +160,6 @@ public:
> fp_contract(LangOpts.DefaultFPContract) {} };
> 
> -/// \brief OpenCL volatile opti

RE: r269431 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

BTW is there a way to turn on this warning with CMake? I could not see this 
warning in my own build.

Thanks.

Sam

-Original Message-
From: Liu, Yaxun (Sam) 
Sent: Friday, May 13, 2016 1:33 PM
To: 'steve...@apple.com' 
Cc: cfe-commits@lists.llvm.org
Subject: RE: r269431 - [OpenCL] Add supported OpenCL extensions to target info.

Thanks Steven.

I have reverted my change. I will apply your patch and re-commit.

Sam

-Original Message-
From: steve...@apple.com [mailto:steve...@apple.com]
Sent: Friday, May 13, 2016 1:27 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits@lists.llvm.org
Subject: Re: r269431 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Yaxun

You seems missing some override keyword that triggers 
-Winconsistent-missing-override. See:
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_build/24442/warnings8Result/new/

Here is a patch to fix them:

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index 
b1b12e4..20f1e95 100644
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -2087,7 +2087,7 @@ public:
 return true;
   }
 
-   void setSupportedOpenCLOpts() {
+   void setSupportedOpenCLOpts() override {
  auto &Opts = getSupportedOpenCLOpts();
  Opts.cl_clang_storage_class_specifiers = 1;
  Opts.cl_khr_gl_sharing = 1;
@@ -2731,7 +2731,7 @@ public:
 return true;
   }
 
-  void setSupportedOpenCLOpts() {
+  void setSupportedOpenCLOpts() override {
 getSupportedOpenCLOpts().setAll();
   }
 };
@@ -7877,7 +7877,7 @@ public:
 return CC_SpirFunction;
   }
 
-  void setSupportedOpenCLOpts() {
+  void setSupportedOpenCLOpts() override {
 // Assume all OpenCL extensions and optional core features are supported
 // for SPIR since it is a generic target.
 getSupportedOpenCLOpts().setAll();

Thanks

Steven

> On May 13, 2016, at 8:44 AM, Yaxun Liu via cfe-commits 
>  wrote:
> 
> Author: yaxunl
> Date: Fri May 13 10:44:37 2016
> New Revision: 269431
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=269431&view=rev
> Log:
> [OpenCL] Add supported OpenCL extensions to target info.
> 
> Add supported OpenCL extensions to target info. It serves as default values 
> to save the users of the burden setting each supported extensions and 
> optional core features in command line.
> 
> Differential Revision: http://reviews.llvm.org/D19484
> 
> Added:
>cfe/trunk/include/clang/Basic/OpenCLOptions.h
>cfe/trunk/test/SemaOpenCL/extensions.cl
> Removed:
>cfe/trunk/test/SemaOpenCL/extension-fp64-cl1.1.cl
>cfe/trunk/test/SemaOpenCL/extension-fp64.cl
>cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl1.2.cl
>cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl2.0.cl
> Modified:
>cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>cfe/trunk/include/clang/Basic/LangOptions.h
>cfe/trunk/include/clang/Basic/OpenCLExtensions.def
>cfe/trunk/include/clang/Basic/TargetInfo.h
>cfe/trunk/include/clang/Basic/TargetOptions.h
>cfe/trunk/lib/Basic/Targets.cpp
>cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>cfe/trunk/lib/Parse/ParsePragma.cpp
>cfe/trunk/lib/Sema/Sema.cpp
>cfe/trunk/test/CodeGenOpenCL/builtins-r600.cl
>cfe/trunk/test/CodeGenOpenCL/fpmath.cl
>cfe/trunk/test/CodeGenOpenCL/half.cl
>cfe/trunk/test/Lexer/opencl-half-literal.cl
>cfe/trunk/test/Misc/languageOptsOpenCL.cl
>cfe/trunk/test/PCH/opencl-extensions.cl
>cfe/trunk/test/Parser/opencl-astype.cl
>cfe/trunk/test/Parser/opencl-atomics-cl20.cl
>cfe/trunk/test/Parser/opencl-pragma.cl
>cfe/trunk/test/Parser/opencl-storage-class.cl
>cfe/trunk/test/SemaOpenCL/half.cl
>cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl
>cfe/trunk/test/SemaOpenCL/invalid-logical-ops-1.2.cl
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diag
> nosticParseKinds.td?rev=269431&r1=269430&r2=269431&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri May 13
> +++ 10:44:37 2016
> @@ -926,6 +926,10 @@ def warn_pragma_expected_enable_disable
>   "expected 'enable' or 'disable' - ignoring">, 
> InGroup; def warn_pragma_unknown_extension : Warning<
>   "unknown OpenCL extension %0 - ignoring">, InGroup;
> +def warn_pragma_unsupported_extension : Warning<
> +  "unsupported OpenCL extension %0 - ignoring">, 
> +InGroup; def warn_pragma_extension_is_core : Warning<
> +  "OpenCL extension %0 is core feature or supported optional core 
> +feature - ignoring">, InGroup;
> 
> // OpenCL errors.
> def err_opencl_taking_function_address_parser : Error<
> 
> Modified: cfe/trunk/include/clang/Basic/LangOptions.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Lang
> Options.h?rev=269431&r1=269430&r2=269431&view=diff
> ==

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Currently Clang does not emit opencl.used.extensions metadata, so the issue 
mentioned does not exist.

Also extensions supported by a target and extensions used by an OpenCL program 
is different concept.

I'd say Clang currently miss a feature to detect used extensions and emit the 
metadata.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Friday, May 20, 2016 3:23 PM
To: Liu, Yaxun (Sam) ; Jeroen Ketema 

Cc: Clang Commits ; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Sam,

Has this been addressed?

@Jeroen, as far as I am aware adding supported extensions is completely new to 
Clang and shouldn't be braking any existing functionality that are related to 
that. Could you elaborate on the problem. Would creating new Clang target help? 
Otherwise, do you have any other proposal for the solution?

Thanks,
Anastasia



From: cfe-commits  on behalf of Jeroen 
Ketema via cfe-commits 
Sent: 17 May 2016 12:49
To: Yaxun Liu via cfe-commits
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi,

The below commit enables all OpenCL extensions for the SPIR target by default. 
This incorrect, as SPIR allows you to specify which extensions are 
enabled/disabled its metadata. This means that any SPIR generated by Clang may 
now be rejected by specific OpenCL platforms, because they might not support 
all extensions.

If possible I would like to see this commit reverted until that problem has 
been addressed.

Regards,

Jeroen

> On 16 May 2016, at 18:06, Yaxun Liu via cfe-commits  lists.llvm.org> wrote:
>
> Author: yaxunl
> Date: Mon May 16 12:06:34 2016
> New Revision: 269670
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269670&view=rev
> Log:
> [OpenCL] Add supported OpenCL extensions to target info.
>
> Add supported OpenCL extensions to target info. It serves as default values 
> to save the users of the burden setting each supported extensions and 
> optional core features in command line.
>
> Re-commit after fixing build error due to missing override attribute.
>
> Differential Revision: http://reviews.llvm.org/D19484
>
> Added:
>   cfe/trunk/include/clang/Basic/OpenCLOptions.h
>   cfe/trunk/test/SemaOpenCL/extensions.cl
> Removed:
>   cfe/trunk/test/SemaOpenCL/extension-fp64-cl1.1.cl
>   cfe/trunk/test/SemaOpenCL/extension-fp64.cl
>   cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl1.2.cl
>   cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl2.0.cl
> Modified:
>   cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>   cfe/trunk/include/clang/Basic/LangOptions.h
>   cfe/trunk/include/clang/Basic/OpenCLExtensions.def
>   cfe/trunk/include/clang/Basic/TargetInfo.h
>   cfe/trunk/include/clang/Basic/TargetOptions.h
>   cfe/trunk/lib/Basic/Targets.cpp
>   cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>   cfe/trunk/lib/Parse/ParsePragma.cpp
>   cfe/trunk/lib/Sema/Sema.cpp
>   cfe/trunk/test/CodeGenOpenCL/builtins-r600.cl
>   cfe/trunk/test/CodeGenOpenCL/fpmath.cl
>   cfe/trunk/test/CodeGenOpenCL/half.cl
>   cfe/trunk/test/Lexer/opencl-half-literal.cl
>   cfe/trunk/test/Misc/languageOptsOpenCL.cl
>   cfe/trunk/test/PCH/opencl-extensions.cl
>   cfe/trunk/test/Parser/opencl-astype.cl
>   cfe/trunk/test/Parser/opencl-atomics-cl20.cl
>   cfe/trunk/test/Parser/opencl-pragma.cl
>   cfe/trunk/test/Parser/opencl-storage-class.cl
>   cfe/trunk/test/SemaOpenCL/half.cl
>   cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl
>   cfe/trunk/test/SemaOpenCL/invalid-logical-ops-1.2.cl
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diag
> nosticParseKinds.td?rev=269670&r1=269669&r2=269670&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon May 16 
> +++ 12:06:34 2016
> @@ -926,6 +926,10 @@ def warn_pragma_expected_enable_disable
>  "expected 'enable' or 'disable' - ignoring">, 
> InGroup; def warn_pragma_unknown_extension : Warning<  
> "unknown OpenCL extension %0 - ignoring">, InGroup;
> +def warn_pragma_unsupported_extension : Warning<
> +  "unsupported OpenCL extension %0 - ignoring">, 
> +InGroup; def warn_pragma_extension_is_core : Warning<
> +  "OpenCL extension %0 is core feature or supported optional core 
> +feature - ignoring">, InGroup;
>
> // OpenCL errors.
> def err_opencl_taking_function_address_parser : Error<
>
> Modified: cfe/trunk/include/clang/Basic/LangOptions.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Lang
> Options.h?rev=269670&r1=269669&r2=269670&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/LangOptions.h (original)
> +++ cfe/trunk/include/clang/Basic/LangOption

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

I think this feature can be implemented by keeping a record of enabled OpenCL 
extensions by user's program.

For optional core feature cl_khr_fp64, we just need to detect if double type is 
used by user's program.

Sam

-Original Message-
From: Liu, Yaxun (Sam) 
Sent: Friday, May 20, 2016 3:45 PM
To: 'Anastasia Stulova' ; Jeroen Ketema 

Cc: Clang Commits ; nd 
Subject: RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Currently Clang does not emit opencl.used.extensions metadata, so the issue 
mentioned does not exist.

Also extensions supported by a target and extensions used by an OpenCL program 
is different concept.

I'd say Clang currently miss a feature to detect used extensions and emit the 
metadata.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com]
Sent: Friday, May 20, 2016 3:23 PM
To: Liu, Yaxun (Sam) ; Jeroen Ketema 

Cc: Clang Commits ; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Sam,

Has this been addressed?

@Jeroen, as far as I am aware adding supported extensions is completely new to 
Clang and shouldn't be braking any existing functionality that are related to 
that. Could you elaborate on the problem. Would creating new Clang target help? 
Otherwise, do you have any other proposal for the solution?

Thanks,
Anastasia



From: cfe-commits  on behalf of Jeroen 
Ketema via cfe-commits 
Sent: 17 May 2016 12:49
To: Yaxun Liu via cfe-commits
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi,

The below commit enables all OpenCL extensions for the SPIR target by default. 
This incorrect, as SPIR allows you to specify which extensions are 
enabled/disabled its metadata. This means that any SPIR generated by Clang may 
now be rejected by specific OpenCL platforms, because they might not support 
all extensions.

If possible I would like to see this commit reverted until that problem has 
been addressed.

Regards,

Jeroen

> On 16 May 2016, at 18:06, Yaxun Liu via cfe-commits  lists.llvm.org> wrote:
>
> Author: yaxunl
> Date: Mon May 16 12:06:34 2016
> New Revision: 269670
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269670&view=rev
> Log:
> [OpenCL] Add supported OpenCL extensions to target info.
>
> Add supported OpenCL extensions to target info. It serves as default values 
> to save the users of the burden setting each supported extensions and 
> optional core features in command line.
>
> Re-commit after fixing build error due to missing override attribute.
>
> Differential Revision: http://reviews.llvm.org/D19484
>
> Added:
>   cfe/trunk/include/clang/Basic/OpenCLOptions.h
>   cfe/trunk/test/SemaOpenCL/extensions.cl
> Removed:
>   cfe/trunk/test/SemaOpenCL/extension-fp64-cl1.1.cl
>   cfe/trunk/test/SemaOpenCL/extension-fp64.cl
>   cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl1.2.cl
>   cfe/trunk/test/SemaOpenCL/optional-core-fp64-cl2.0.cl
> Modified:
>   cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>   cfe/trunk/include/clang/Basic/LangOptions.h
>   cfe/trunk/include/clang/Basic/OpenCLExtensions.def
>   cfe/trunk/include/clang/Basic/TargetInfo.h
>   cfe/trunk/include/clang/Basic/TargetOptions.h
>   cfe/trunk/lib/Basic/Targets.cpp
>   cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>   cfe/trunk/lib/Parse/ParsePragma.cpp
>   cfe/trunk/lib/Sema/Sema.cpp
>   cfe/trunk/test/CodeGenOpenCL/builtins-r600.cl
>   cfe/trunk/test/CodeGenOpenCL/fpmath.cl
>   cfe/trunk/test/CodeGenOpenCL/half.cl
>   cfe/trunk/test/Lexer/opencl-half-literal.cl
>   cfe/trunk/test/Misc/languageOptsOpenCL.cl
>   cfe/trunk/test/PCH/opencl-extensions.cl
>   cfe/trunk/test/Parser/opencl-astype.cl
>   cfe/trunk/test/Parser/opencl-atomics-cl20.cl
>   cfe/trunk/test/Parser/opencl-pragma.cl
>   cfe/trunk/test/Parser/opencl-storage-class.cl
>   cfe/trunk/test/SemaOpenCL/half.cl
>   cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl
>   cfe/trunk/test/SemaOpenCL/invalid-logical-ops-1.2.cl
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diag
> nosticParseKinds.td?rev=269670&r1=269669&r2=269670&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon May 16
> +++ 12:06:34 2016
> @@ -926,6 +926,10 @@ def warn_pragma_expected_enable_disable
>  "expected 'enable' or 'disable' - ignoring">, 
> InGroup; def warn_pragma_unknown_extension : Warning< 
> "unknown OpenCL extension %0 - ignoring">, InGroup;
> +def warn_pragma_unsupported_extension : Warning<
> +  "unsupported OpenCL extension %0 - ignoring">, 
> +InGroup; def warn_pragma_extension_is_core : Warning<
> +  "OpenCL extension %0 is core feature or supported optional core 
> +feature - ignoring">, InGroup;
>
> // Open

RE: r267590 - [OpenCL] Add predefined macros.

[Liu, Yaxun (Sam) via cfe-commits]

Did clang accept -std=CL2.0 before this change?

According to this diff, the only accepted option for -std= is c99 when 
compiling OpenCL programs.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Tuesday, May 24, 2016 2:48 PM
To: Liu, Yaxun (Sam) ; cfe-commits@lists.llvm.org
Cc: nd 
Subject: RE: r267590 - [OpenCL] Add predefined macros.

Hi Sam,

I think this commit broke Clang.

It seems we are no longer able to pass the -std=CL2.0, which is important for 
the standalone Clang OpenCL users as -cl-std is frontend only option.
 
clang  -std=CL2.0 test.cl
error: invalid argument '-std=CL2.0' not allowed with 'OpenCL'

We might have to revert or rework this bit:

--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Apr 26 14:25:46
+++ 2016
@@ -1495,9 +1495,8 @@ static void ParseLangArgs(LangOptions &O
 << A->getAsString(Args) << "C++/ObjC++";
 break;
   case IK_OpenCL:
-if (!Std.isC99())
-  Diags.Report(diag::err_drv_argument_not_allowed_with)
-<< A->getAsString(Args) << "OpenCL";
+Diags.Report(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "OpenCL";
 break;
   case IK_CUDA:
   case IK_PreprocessedCuda:

Do you think you could look into this?

Thanks,
Anastasia

-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Yaxun Liu via cfe-commits
Sent: 26 April 2016 20:26
To: cfe-commits@lists.llvm.org
Subject: r267590 - [OpenCL] Add predefined macros.

Author: yaxunl
Date: Tue Apr 26 14:25:46 2016
New Revision: 267590

URL: http://llvm.org/viewvc/llvm-project?rev=267590&view=rev
Log:
[OpenCL] Add predefined macros.

OpenCL spec requires __OPENCL_C_VERSION__ to be defined based on -cl-std 
option. This patch implements that.

The patch also defines __FAST_RELAXED_MATH__ based on -cl-fast-relaxed-math 
option.

Also fixed a test using -std=c99 for OpenCL program. Limit allowed language 
standard of OpenCL to be OpenCL standards.

Differential Revision: http://reviews.llvm.org/D19071

Removed:
cfe/trunk/test/Frontend/std.cl
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/Frontend/stdlang.c
cfe/trunk/test/Preprocessor/predefined-macros.c

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=267590&r1=267589&r2=267590&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Apr 26 14:25:46
+++ 2016
@@ -1495,9 +1495,8 @@ static void ParseLangArgs(LangOptions &O
 << A->getAsString(Args) << "C++/ObjC++";
 break;
   case IK_OpenCL:
-if (!Std.isC99())
-  Diags.Report(diag::err_drv_argument_not_allowed_with)
-<< A->getAsString(Args) << "OpenCL";
+Diags.Report(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "OpenCL";
 break;
   case IK_CUDA:
   case IK_PreprocessedCuda:

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=267590&r1=267589&r2=267590&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Tue Apr 26 14:25:46 2016
@@ -408,6 +408,39 @@ static void InitializeStandardPredefined
   if (LangOpts.ObjC1)
 Builder.defineMacro("__OBJC__");
 
+  // OpenCL v1.0/1.1 s6.9, v1.2/2.0 s6.10: Preprocessor Directives and Macros.
+  if (LangOpts.OpenCL) {
+// OpenCL v1.0 and v1.1 do not have a predefined macro to indicate the
+// language standard with which the program is compiled. __OPENCL_VERSION__
+// is for the OpenCL version supported by the OpenCL device, which is not
+// necessarily the language standard with which the program is compiled.
+// A shared OpenCL header file requires a macro to indicate the language
+// standard. As a workaround, __OPENCL_C_VERSION__ is defined for
+// OpenCL v1.0 and v1.1.
+switch (LangOpts.OpenCLVersion) {
+case 100:
+  Builder.defineMacro("__OPENCL_C_VERSION__", "100");
+  break;
+case 110:
+  Builder.defineMacro("__OPENCL_C_VERSION__", "110");
+  break;
+case 120:
+  Builder.defineMacro("__OPENCL_C_VERSION__", "120");
+  break;
+case 200:
+  Builder.defineMacro("__OPENCL_C_VERSION__", "200");
+  break;
+default:
+  llvm_unreachable("Unsupported OpenCL version");
+}
+Builder.defineMacro("CL_VERSION_1_0", "100");
+Builder.defineMacro("CL_VERSION_1_1", "110");
+Build

RE: r267590 - [OpenCL] Add predefined macros.

[Liu, Yaxun (Sam) via cfe-commits]

I will take a look. Thanks.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Wednesday, May 25, 2016 10:29 AM
To: Liu, Yaxun (Sam) ; cfe-commits@lists.llvm.org
Cc: nd 
Subject: RE: r267590 - [OpenCL] Add predefined macros.

This is because OpenCL sets C99 flag as being superset of it. If you check in 
clang/Frontend/LangStandards.def

LANGSTANDARD(opencl, "cl",
 "OpenCL 1.0",
 LineComment | C99 | Digraphs | HexFloat)

We should undo this change as it leaves no possibility to specify an OpenCL 
version during compilation apart from using the frontend option (which isn't 
conventional approach).

Anastasia

-Original Message-
From: Liu, Yaxun (Sam) [mailto:yaxun@amd.com]
Sent: 24 May 2016 19:58
To: Anastasia Stulova; cfe-commits@lists.llvm.org
Cc: nd
Subject: RE: r267590 - [OpenCL] Add predefined macros.

Did clang accept -std=CL2.0 before this change?

According to this diff, the only accepted option for -std= is c99 when 
compiling OpenCL programs.

Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com]
Sent: Tuesday, May 24, 2016 2:48 PM
To: Liu, Yaxun (Sam) ; cfe-commits@lists.llvm.org
Cc: nd 
Subject: RE: r267590 - [OpenCL] Add predefined macros.

Hi Sam,

I think this commit broke Clang.

It seems we are no longer able to pass the -std=CL2.0, which is important for 
the standalone Clang OpenCL users as -cl-std is frontend only option.
 
clang  -std=CL2.0 test.cl
error: invalid argument '-std=CL2.0' not allowed with 'OpenCL'

We might have to revert or rework this bit:

--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Apr 26 14:25:46
+++ 2016
@@ -1495,9 +1495,8 @@ static void ParseLangArgs(LangOptions &O
 << A->getAsString(Args) << "C++/ObjC++";
 break;
   case IK_OpenCL:
-if (!Std.isC99())
-  Diags.Report(diag::err_drv_argument_not_allowed_with)
-<< A->getAsString(Args) << "OpenCL";
+Diags.Report(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "OpenCL";
 break;
   case IK_CUDA:
   case IK_PreprocessedCuda:

Do you think you could look into this?

Thanks,
Anastasia

-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Yaxun Liu via cfe-commits
Sent: 26 April 2016 20:26
To: cfe-commits@lists.llvm.org
Subject: r267590 - [OpenCL] Add predefined macros.

Author: yaxunl
Date: Tue Apr 26 14:25:46 2016
New Revision: 267590

URL: http://llvm.org/viewvc/llvm-project?rev=267590&view=rev
Log:
[OpenCL] Add predefined macros.

OpenCL spec requires __OPENCL_C_VERSION__ to be defined based on -cl-std 
option. This patch implements that.

The patch also defines __FAST_RELAXED_MATH__ based on -cl-fast-relaxed-math 
option.

Also fixed a test using -std=c99 for OpenCL program. Limit allowed language 
standard of OpenCL to be OpenCL standards.

Differential Revision: http://reviews.llvm.org/D19071

Removed:
cfe/trunk/test/Frontend/std.cl
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/Frontend/stdlang.c
cfe/trunk/test/Preprocessor/predefined-macros.c

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=267590&r1=267589&r2=267590&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Apr 26 14:25:46
+++ 2016
@@ -1495,9 +1495,8 @@ static void ParseLangArgs(LangOptions &O
 << A->getAsString(Args) << "C++/ObjC++";
 break;
   case IK_OpenCL:
-if (!Std.isC99())
-  Diags.Report(diag::err_drv_argument_not_allowed_with)
-<< A->getAsString(Args) << "OpenCL";
+Diags.Report(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "OpenCL";
 break;
   case IK_CUDA:
   case IK_PreprocessedCuda:

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=267590&r1=267589&r2=267590&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Tue Apr 26 14:25:46 2016
@@ -408,6 +408,39 @@ static void InitializeStandardPredefined
   if (LangOpts.ObjC1)
 Builder.defineMacro("__OBJC__");
 
+  // OpenCL v1.0/1.1 s6.9, v1.2/2.0 s6.10: Preprocessor Directives and Macros.
+  if (LangOpts.OpenCL) {
+// OpenCL v1.0 and v1.1 do not have a predefined macro to indicate the
+// language standard with which the program is compiled. __OPENCL_VERSION__
+// is for 

RE: [PATCH] D20630: [OpenCL] Allow -std=CL|CL1.1|CL1.2|CL2.0 in driver

[Liu, Yaxun (Sam) via cfe-commits]

I will fix that. Thanks.

Sam

From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith
Sent: Wednesday, May 25, 2016 2:54 PM
To: Liu, Yaxun (Sam) ; 
reviews+d20630+public+1c58d99d1f368...@reviews.llvm.org
Cc: alexey.ba...@intel.com; Anastasia Stulova ; 
cfe-commits 
Subject: Re: [PATCH] D20630: [OpenCL] Allow -std=CL|CL1.1|CL1.2|CL2.0 in driver


On 25 May 2016 9:13 a.m., "Yaxun Liu via cfe-commits" 
mailto:cfe-commits@lists.llvm.org>> wrote:
>
> yaxunl created this revision.
> yaxunl added a reviewer: Anastasia.
> yaxunl added subscribers: pxli168, bader, tstellarAMD, cfe-commits.
>
> Fix a regression which forbids using -std=CL|CL1.1|CL1.2|CL2.0 in driver.
>
> Changed -std=cl to -std=CL to match -cl-std=CL.

This is inconsistent with our existing command-line interface. It would be more 
consistent to change -cl-std= to accept lowercase cl instead.

> http://reviews.llvm.org/D20630
>
> Files:
>   include/clang/Frontend/LangStandards.def
>   lib/Frontend/CompilerInvocation.cpp
>   test/Driver/opencl.cl
>
> Index: test/Driver/opencl.cl
> ===
> --- /dev/null
> +++ test/Driver/opencl.cl
> @@ -0,0 +1,11 @@
> +// RUN: %clang %s
> +// RUN: %clang -std=CL %s
> +// RUN: %clang -std=CL1.1 %s
> +// RUN: %clang -std=CL1.2 %s
> +// RUN: %clang -std=CL2.0 %s
> +// RUN: not %clang_cc1 -std=c99 -DOPENCL %s 2>&1 | FileCheck 
> --check-prefix=CHECK-C99 %s
> +// RUN: not %clang_cc1 -std=invalid -DOPENCL %s 2>&1 | FileCheck 
> --check-prefix=CHECK-INVALID %s
> +// CHECK-C99: error: invalid argument '-std=c99' not allowed with 'OpenCL'
> +// CHECK-INVALID: error: invalid value 'invalid' in '-std=invalid'
> +
> +kernel void func(void);
> Index: lib/Frontend/CompilerInvocation.cpp
> ===
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -1398,6 +1398,13 @@
>  Opts.AddVFSOverlayFile(A->getValue());
>  }
>
> +bool isOpenCL(LangStandard::Kind LangStd) {
> +  return LangStd == LangStandard::lang_opencl
> +|| LangStd == LangStandard::lang_opencl11
> +|| LangStd == LangStandard::lang_opencl12
> +|| LangStd == LangStandard::lang_opencl20;
> +}
> +
>  void CompilerInvocation::setLangDefaults(LangOptions &Opts, InputKind IK,
>   const llvm::Triple &T,
>   LangStandard::Kind LangStd) {
> @@ -1462,7 +1469,7 @@
>Opts.ImplicitInt = Std.hasImplicitInt();
>
>// Set OpenCL Version.
> -  Opts.OpenCL = LangStd == LangStandard::lang_opencl || IK == IK_OpenCL;
> +  Opts.OpenCL = isOpenCL(LangStd) || IK == IK_OpenCL;
>if (LangStd == LangStandard::lang_opencl)
>  Opts.OpenCLVersion = 100;
>else if (LangStd == LangStandard::lang_opencl11)
> @@ -1555,8 +1562,9 @@
>  << A->getAsString(Args) << "C++/ObjC++";
>  break;
>case IK_OpenCL:
> -Diags.Report(diag::err_drv_argument_not_allowed_with)
> -  << A->getAsString(Args) << "OpenCL";
> +if (!isOpenCL(LangStd))
> +  Diags.Report(diag::err_drv_argument_not_allowed_with)
> +<< A->getAsString(Args) << "OpenCL";
>  break;
>case IK_CUDA:
>case IK_PreprocessedCuda:
> Index: include/clang/Frontend/LangStandards.def
> ===
> --- include/clang/Frontend/LangStandards.def
> +++ include/clang/Frontend/LangStandards.def
> @@ -132,7 +132,7 @@
>   Digraphs | HexFloat | GNUMode)
>
>  // OpenCL
> -LANGSTANDARD(opencl, "cl",
> +LANGSTANDARD(opencl, "CL",
>   "OpenCL 1.0",
>   LineComment | C99 | Digraphs | HexFloat)
>  LANGSTANDARD(opencl11, "CL1.1",
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D20681: Add target-specific pre-linking passes to Clang

[Liu, Yaxun (Sam) via cfe-commits]

+ Brian

-Original Message-
From: Tom Stellard [mailto:thomas.stell...@amd.com] 
Sent: Thursday, May 26, 2016 11:11 AM
To: Liu, Yaxun (Sam) ; rich...@metafoo.co.uk; 
anastasia.stul...@arm.com
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D20681: Add target-specific pre-linking passes to Clang

tstellarAMD added a comment.

Can you give some examples of what pre-link passes may be required?


http://reviews.llvm.org/D20681



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

RE: [PATCH] D20681: Add target-specific pre-linking passes to Clang

[Liu, Yaxun (Sam) via cfe-commits]

+Jeff

To cite some of the previous discussions 
(http://lists.llvm.org/pipermail/cfe-dev/2016-May/048822.html )

Brian:

On our side, we use such pre-link passes to interface with the specifics of our 
built-in function library.  For example, we transform printf calls into a form 
that interacts with our library.  We also transform calls to pipe functions, 
kernel enqueue related functions, and transform calls to the atomic functions 
to corresponding LLVM atomic instructions (and keep track of the memory scope 
elsewhere currently).  You may have noticed that we have a proposal out to 
enable the atomic instructions to directly carry a memory scope.

Jeff:

We have similar passes related to builtin functions but they are rather 
specific to our implementation and not too complex.

Thanks.

Sam


-Original Message-
From: Liu, Yaxun (Sam) 
Sent: Thursday, May 26, 2016 11:44 AM
To: reviews+d20681+public+7364792746786...@reviews.llvm.org; 
rich...@metafoo.co.uk; anastasia.stul...@arm.com
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org; 
Sumner, Brian 
Subject: RE: [PATCH] D20681: Add target-specific pre-linking passes to Clang

+ Brian

-Original Message-
From: Tom Stellard [mailto:thomas.stell...@amd.com] 
Sent: Thursday, May 26, 2016 11:11 AM
To: Liu, Yaxun (Sam) ; rich...@metafoo.co.uk; 
anastasia.stul...@arm.com
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D20681: Add target-specific pre-linking passes to Clang

tstellarAMD added a comment.

Can you give some examples of what pre-link passes may be required?


http://reviews.llvm.org/D20681



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

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Hi Jeroen,

OpenCL spec v1.1 s9.1 says

Every extension which affects the OpenCL language semantics, syntax or adds 
built-in functions
to the language must create a preprocessor #define that matches the extension 
name string.
This #define would be available in the language if and only if the extension is 
supported on a
given implementation.

The spec does not say pragma enabling/disabling an extension will 
define/undefined the macro.

Also before this commit, Clang did not define/undefine the macro based on 
pragma either.

You could define/undefined your own macro to indicate an extension is 
enabled/disabled then use it to condition your code.

Thanks.

Sam

-Original Message-
From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk] 
Sent: Monday, May 30, 2016 10:08 AM
To: Anastasia Stulova 
Cc: Liu, Yaxun (Sam) ; Clang Commits 
; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.


Hi Anastasia,

My apologies for my slow reply. My main issue was that the defaults have 
changed, which is somewhat annoying. However, digging deeper into this I'm 
noticing two serious problems:

* When I do:

#pragma OPENCL EXTENSION cl_khr_fp16 : disable

the cl_khr_fp16 macro stays defined in code I compile down to the SPIR target, 
which means I cannot do conditional compilation based on which extensions are 
enabled/disabled. This means I now need to start pulling additional manual 
tricks to do conditional compilation of half precision code.

[Sam] Before this change, clang did not define/undefine cl_khr_fp16 based on 
pragma. How could your code work?

* If I understand the patch correctly, it allows me to enable extensions for 
targets that do not support it. There does not seem a check against the 
initially enabled extensions, just a check for what is or isn't supported in a 
particular version of OpenCL. This means I can now, e.g., enable sharing with 
Direct3D 11 for the nvptx target even though this target does not support this 
functionality.

On top of the above, I do not understand why this patch introduces code for 
extensions like ICD, which is an OpenCL API concept and not a OpenCL C concept.
[Sam] The spec requires 

Jeroen

> On 20 May 2016, at 21:01, Anastasia Stulova  wrote:
> 
> Thanks Sam!
> 
> @Jeroen, could you give us more details about your problem.
> 
> Anastasia
> 
> -Original Message-
> From: Liu, Yaxun (Sam) [mailto:yaxun@amd.com]
> Sent: 20 May 2016 20:52
> To: Anastasia Stulova; Jeroen Ketema
> Cc: Clang Commits; nd
> Subject: RE: r269670 - [OpenCL] Add supported OpenCL extensions to target 
> info.
> 
> I think this feature can be implemented by keeping a record of enabled OpenCL 
> extensions by user's program.
> 
> For optional core feature cl_khr_fp64, we just need to detect if double type 
> is used by user's program.
> 
> Sam
> 
> -Original Message-
> From: Liu, Yaxun (Sam)
> Sent: Friday, May 20, 2016 3:45 PM
> To: 'Anastasia Stulova' ; Jeroen Ketema 
> 
> Cc: Clang Commits ; nd 
> Subject: RE: r269670 - [OpenCL] Add supported OpenCL extensions to target 
> info.
> 
> Currently Clang does not emit opencl.used.extensions metadata, so the issue 
> mentioned does not exist.
> 
> Also extensions supported by a target and extensions used by an OpenCL 
> program is different concept.
> 
> I'd say Clang currently miss a feature to detect used extensions and emit the 
> metadata.
> 
> Sam
> 
> -Original Message-
> From: Anastasia Stulova [mailto:anastasia.stul...@arm.com]
> Sent: Friday, May 20, 2016 3:23 PM
> To: Liu, Yaxun (Sam) ; Jeroen Ketema 
> 
> Cc: Clang Commits ; nd 
> Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target 
> info.
> 
> Hi Sam,
> 
> Has this been addressed?
> 
> @Jeroen, as far as I am aware adding supported extensions is completely new 
> to Clang and shouldn't be braking any existing functionality that are related 
> to that. Could you elaborate on the problem. Would creating new Clang target 
> help? Otherwise, do you have any other proposal for the solution?
> 
> Thanks,
> Anastasia
> 
> 
> 
> From: cfe-commits  on behalf of 
> Jeroen Ketema via cfe-commits 
> Sent: 17 May 2016 12:49
> To: Yaxun Liu via cfe-commits
> Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target 
> info.
> 
> Hi,
> 
> The below commit enables all OpenCL extensions for the SPIR target by 
> default. This incorrect, as SPIR allows you to specify which extensions are 
> enabled/disabled its metadata. This means that any SPIR generated by Clang 
> may now be rejected by specific OpenCL platforms, because they might not 
> support all extensions.
> 
> If possible I would like to see this commit reverted until that problem has 
> been addressed.
> 
> Regards,
> 
> Jeroen
> 
>> On 16 May 2016, at 18:06, Yaxun Liu via cfe-commits > lists.llvm.org> wrote:
>> 
>> Author: yaxunl
>> Date: Mon May 16 12:06:34 2016
>> New Revision: 269670
>> 
>> URL: http:

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Hi Jeroen,

I added all extensions since it may be useful for users to know that certain 
extensions are supported by a platform. The spec does not specify those options 
which do not affect language semantics should or should not be defined. I am 
considering removing them since they may encourage users to depend on a feature 
not supported by other OpenCL compiler.

This commit does not change the initial state of the extensions. An extension 
is supported is not the same as enabled. At the beginning all extensions are 
disabled.

Each target has its own supported extensions except SPIR. Since it is a generic 
target which is supposed to run on different platforms supporting it. It is 
assumed that all extensions are supported. For some targets the supported 
extensions may not be accurate currently, but should be updated to reflect 
their real situation.

Sam

-Original Message-
From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk] 
Sent: Tuesday, May 31, 2016 5:36 PM
To: Liu, Yaxun (Sam) 
Cc: Anastasia Stulova ; Clang Commits 
; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.


By the way, are you accidentally violating:


The initial state of the compiler is as if the directive #pragma OPENCL 
EXTENSION all : disable was issued, telling the compiler that all error and 
warning reporting must be done according to this specification, ignoring any 
extensions.


That from just before the text quoted below.

Jeroen

> On 31 May 2016, at 22:30, Jeroen Ketema  wrote:
> 
> 
> Hi Sam,
> 
> Fair enough. However, if I read your quote correctly, then there is 
> absolutely no need to define a cl_khr_icd, pragma, as ICD in no way affects 
> the language. Why do you define one anyway?
> 
> The following also hasn’t been cleared up yet:
> 
> 
> * If I understand the patch correctly, it allows me to enable extensions for 
> targets that do not support it. There does not seem a check against the 
> initially enabled extensions, just a check for what is or isn't supported in 
> a particular version of OpenCL. This means I can now, e.g., enable sharing 
> with Direct3D 11 for the nvptx target even though this target does not 
> support this functionality.
> 
> 
> Could you please clarify?
> 
> Thanks,
> 
> Jeroen
> 
>> On 31 May 2016, at 21:29, Liu, Yaxun (Sam)  wrote:
>> 
>> Hi Jeroen,
>> 
>> OpenCL spec v1.1 s9.1 says
>> 
>> Every extension which affects the OpenCL language semantics, syntax 
>> or adds built-in functions to the language must create a preprocessor 
>> #define that matches the extension name string.
>> This #define would be available in the language if and only if the 
>> extension is supported on a given implementation.
>> 
>> The spec does not say pragma enabling/disabling an extension will 
>> define/undefined the macro.
>> 
>> Also before this commit, Clang did not define/undefine the macro based on 
>> pragma either.
>> 
>> You could define/undefined your own macro to indicate an extension is 
>> enabled/disabled then use it to condition your code.
>> 
>> Thanks.
>> 
>> Sam
>> 
>> -Original Message-
>> From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk]
>> Sent: Monday, May 30, 2016 10:08 AM
>> To: Anastasia Stulova 
>> Cc: Liu, Yaxun (Sam) ; Clang Commits 
>> ; nd 
>> Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target 
>> info.
>> 
>> 
>> Hi Anastasia,
>> 
>> My apologies for my slow reply. My main issue was that the defaults have 
>> changed, which is somewhat annoying. However, digging deeper into this I'm 
>> noticing two serious problems:
>> 
>> * When I do:
>> 
>> #pragma OPENCL EXTENSION cl_khr_fp16 : disable
>> 
>> the cl_khr_fp16 macro stays defined in code I compile down to the SPIR 
>> target, which means I cannot do conditional compilation based on which 
>> extensions are enabled/disabled. This means I now need to start pulling 
>> additional manual tricks to do conditional compilation of half precision 
>> code.
>> 
>> [Sam] Before this change, clang did not define/undefine cl_khr_fp16 based on 
>> pragma. How could your code work?
>> 
>> * If I understand the patch correctly, it allows me to enable extensions for 
>> targets that do not support it. There does not seem a check against the 
>> initially enabled extensions, just a check for what is or isn't supported in 
>> a particular version of OpenCL. This means I can now, e.g., enable sharing 
>> with Direct3D 11 for the nvptx target even though this target does not 
>> support this functionality.
>> 
>> On top of the above, I do not understand why this patch introduces code for 
>> extensions like ICD, which is an OpenCL API concept and not a OpenCL C 
>> concept.
>> [Sam] The spec requires
>> 
>> Jeroen
>> 
>>> On 20 May 2016, at 21:01, Anastasia Stulova  
>>> wrote:
>>> 
>>> Thanks Sam!
>>> 
>>> @Jeroen, could you give us more details about your problem.
>>> 
>>> Anastasia
>>> 
>>> -Original Message-
>>> From: Liu, Yaxun (Sam) [mailto:yaxun@

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Sorry for the delay.

In ParsePragma.cpp:

void Parser::HandlePragmaOpenCLExtension() {
  assert(Tok.is(tok::annot_pragma_opencl_extension));
  OpenCLExtData data =
  OpenCLExtData::getFromOpaqueValue(Tok.getAnnotationValue());
  unsigned state = data.getInt();
  IdentifierInfo *ename = data.getPointer();
  SourceLocation NameLoc = Tok.getLocation();
  ConsumeToken(); // The annotation token.

  OpenCLOptions &f = Actions.getOpenCLOptions();
  auto CLVer = getLangOpts().OpenCLVersion;
  auto &Supp = getTargetInfo().getSupportedOpenCLOpts();
  // OpenCL 1.1 9.1: "The all variant sets the behavior for all extensions,
  // overriding all previously issued extension directives, but only if the
  // behavior is set to disable."
  if (state == 0 && ename->isStr("all")) {
#define OPENCLEXT(nm) \
if (Supp.is_##nm##_supported_extension(CLVer)) \
  f.nm = 0;
#include "clang/Basic/OpenCLExtensions.def"
  }
#define OPENCLEXT(nm) else if (ename->isStr(#nm)) \
   if (Supp.is_##nm##_supported_extension(CLVer)) \
 f.nm = state; \
   else if (Supp.is_##nm##_supported_core(CLVer)) \
 PP.Diag(NameLoc, diag::warn_pragma_extension_is_core) << ename; \
   else \
 PP.Diag(NameLoc, diag::warn_pragma_unsupported_extension) << ename;
#include "clang/Basic/OpenCLExtensions.def"
  else {
PP.Diag(NameLoc, diag::warn_pragma_unknown_extension) << ename;
return;
  }
}

Whether an extension is supported is represented by 
getTargetInfo().getSupportedOpenCLOpts(), which does not change with pragma.

Whether an extension is enabled is reprented by Actions.getOpenCLOptions(), 
which changes with pragma.

test/SemaOpenCL/extensions.cl contains examples of unsupported extensions.

Sam

-Original Message-
From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk] 
Sent: Tuesday, May 31, 2016 6:07 PM
To: Liu, Yaxun (Sam) 
Cc: Anastasia Stulova ; Clang Commits 
; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Sam,

> This commit does not change the initial state of the extensions. An extension 
> is supported is not the same as enabled. At the beginning all extensions are 
> disabled.

I do not see this reflected in the code at all. Could you please:

a. Point me to the location where this distinction is made.

b. Convince me that I cannot enable an extension for a target if that target 
does not support the extension?

Jeroen

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

RE: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

[Liu, Yaxun (Sam) via cfe-commits]

Hi Jeroen,

Thanks for your consideration.

I am OK with removing those extensions which do not affect language or builtin 
functions. After removing them, they will be unknown to Clang. If a user tries 
to enable them, a warning will be emitted saying the extension is unknown. 

Anastasia, are you OK with that? Thanks.

Sam

-Original Message-
From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk] 
Sent: Monday, June 6, 2016 5:16 PM
To: Liu, Yaxun (Sam) 
Cc: Anastasia Stulova ; Clang Commits 
; nd 
Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target info.

Hi Sam,

Thanks baring with me, and thanks a lot for your explanation! I indeed now 
believe that this is correct; I got somewhat confused by all the macro magic 
that is going on, and had missed that the enabled extensions are actually 
stored in a class instance separate from the one with available options.

I still think it would be good if the extensions that only affect the host-side 
are removed, in particular cl_khr_icd and cl_khr_terminate_context. We have a 
runtime that indirectly includes both Clang’s OpenCLExtensions.def and the 
Khronos extension header, and we run into problems when we include the 
extension header before OpenCLExtensions.def, because the extension header 
defines macros named cl_khr_icd and cl_khr_terminate_context. I admit this is a 
somewhat special use-case though.

Thanks again,

 Jeroen

> On 02 Jun 2016, at 21:53, Liu, Yaxun (Sam)  wrote:
> 
> Sorry for the delay.
> 
> In ParsePragma.cpp:
> 
> void Parser::HandlePragmaOpenCLExtension() {  
> assert(Tok.is(tok::annot_pragma_opencl_extension));
>  OpenCLExtData data =
>  OpenCLExtData::getFromOpaqueValue(Tok.getAnnotationValue());
>  unsigned state = data.getInt();
>  IdentifierInfo *ename = data.getPointer();  SourceLocation NameLoc = 
> Tok.getLocation();  ConsumeToken(); // The annotation token.
> 
>  OpenCLOptions &f = Actions.getOpenCLOptions();  auto CLVer = 
> getLangOpts().OpenCLVersion;  auto &Supp = 
> getTargetInfo().getSupportedOpenCLOpts();
>  // OpenCL 1.1 9.1: "The all variant sets the behavior for all 
> extensions,  // overriding all previously issued extension directives, 
> but only if the  // behavior is set to disable."
>  if (state == 0 && ename->isStr("all")) { #define OPENCLEXT(nm) \
>if (Supp.is_##nm##_supported_extension(CLVer)) \
>  f.nm = 0;
> #include "clang/Basic/OpenCLExtensions.def"
>  }
> #define OPENCLEXT(nm) else if (ename->isStr(#nm)) \
>   if (Supp.is_##nm##_supported_extension(CLVer)) \
> f.nm = state; \
>   else if (Supp.is_##nm##_supported_core(CLVer)) \
> PP.Diag(NameLoc, diag::warn_pragma_extension_is_core) << ename; \
>   else \
> PP.Diag(NameLoc, diag::warn_pragma_unsupported_extension) << 
> ename; #include "clang/Basic/OpenCLExtensions.def"
>  else {
>PP.Diag(NameLoc, diag::warn_pragma_unknown_extension) << ename;
>return;
>  }
> }
> 
> Whether an extension is supported is represented by 
> getTargetInfo().getSupportedOpenCLOpts(), which does not change with pragma.
> 
> Whether an extension is enabled is reprented by Actions.getOpenCLOptions(), 
> which changes with pragma.
> 
> test/SemaOpenCL/extensions.cl contains examples of unsupported extensions.
> 
> Sam
> 
> -Original Message-
> From: Jeroen Ketema [mailto:j.ket...@imperial.ac.uk]
> Sent: Tuesday, May 31, 2016 6:07 PM
> To: Liu, Yaxun (Sam) 
> Cc: Anastasia Stulova ; Clang Commits 
> ; nd 
> Subject: Re: r269670 - [OpenCL] Add supported OpenCL extensions to target 
> info.
> 
> Hi Sam,
> 
>> This commit does not change the initial state of the extensions. An 
>> extension is supported is not the same as enabled. At the beginning all 
>> extensions are disabled.
> 
> I do not see this reflected in the code at all. Could you please:
> 
> a. Point me to the location where this distinction is made.
> 
> b. Convince me that I cannot enable an extension for a target if that target 
> does not support the extension?
> 
> Jeroen
> 

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

RE: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

[Liu, Yaxun (Sam) via cfe-commits]

$ "chmod" "u-w"
"C:\cygwin64\home\ismail\src\llvm\dist\tools\clang\test\Headers\Output\opencl-c-header.cl.tmp/*"
# command stderr:
r.cl.tmp/*: invalid mode: 'mp/*'

the error msg is strange. On my Cygwin I got different error:
chmod: cannot access 
'C:\cygwin64\home\ismail\src\llvm\dist\tools\clang\test\Headers\Output\opencl-c-header.cl.tmp/*':
 No such file or directory

This is because * is not expanded when quoted.

Sam

-Original Message-
From: Ismail Donmez [mailto:ism...@i10z.com] 
Sent: Wednesday, June 22, 2016 3:11 AM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits 
Subject: Re: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

Hi,

On Mon, Jun 20, 2016 at 10:26 PM, Yaxun Liu via cfe-commits 
 wrote:
> Author: yaxunl
> Date: Mon Jun 20 14:26:00 2016
> New Revision: 273191
>
> URL: http://llvm.org/viewvc/llvm-project?rev=273191&view=rev
> Log:
> [OpenCL] Include opencl-c.h by default as a clang module
>
> Include opencl-c.h by default as a module to utilize the automatic AST 
> caching mechanism of clang modules.
>
> Add an option -finclude-default-header to enable default header for OpenCL, 
> which is off by default.
>
> Differential Revision: http://reviews.llvm.org/D20444
>
> Modified:
> cfe/trunk/include/clang/Basic/LangOptions.def
> cfe/trunk/include/clang/Driver/CC1Options.td
> cfe/trunk/include/clang/Frontend/CompilerInvocation.h
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/lib/Headers/module.modulemap
> cfe/trunk/test/Headers/opencl-c-header.cl

chmod lines doesn't seem to work on Cygwin:

$ "chmod" "u-w"
"C:\cygwin64\home\ismail\src\llvm\dist\tools\clang\test\Headers\Output\opencl-c-header.cl.tmp/*"
# command stderr:
r.cl.tmp/*: invalid mode: 'mp/*'
Try 'r.cl.tmp/* --help' for more information.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

[Liu, Yaxun (Sam) via cfe-commits]

The cmake of Cygwin itself does not support llvm. Which cmake did you use on 
Cygwin? Thanks.

Sam

-Original Message-
From: Ismail Donmez [mailto:ism...@i10z.com] 
Sent: Wednesday, June 22, 2016 12:37 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits 
Subject: Re: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

On Wed, Jun 22, 2016 at 5:45 PM, Liu, Yaxun (Sam)  wrote:
> $ "chmod" "u-w"
> "C:\cygwin64\home\ismail\src\llvm\dist\tools\clang\test\Headers\Output\opencl-c-header.cl.tmp/*"
> # command stderr:
> r.cl.tmp/*: invalid mode: 'mp/*'
>
> the error msg is strange. On my Cygwin I got different error:
> chmod: cannot access 
> 'C:\cygwin64\home\ismail\src\llvm\dist\tools\clang\test\Headers\Output
> \opencl-c-header.cl.tmp/*': No such file or directory
>
> This is because * is not expanded when quoted.

Actually I added the quotes to test if it fixes the issue. Since you have 
cygwin can you reproduce the error?

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

RE: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

[Liu, Yaxun (Sam) via cfe-commits]

I have a patch which may workaround this issue, however I could not test it in 
Cygwin since I got issues build latest cmake in Cygwin and the downloaded cmake 
does not work.

Could you please try it?

Thanks.

Sam

-Original Message-
From: Ismail Donmez [mailto:ism...@i10z.com] 
Sent: Wednesday, June 22, 2016 1:52 PM
To: Liu, Yaxun (Sam) 
Cc: cfe-commits 
Subject: Re: r273191 - [OpenCL] Include opencl-c.h by default as a clang module

Hi,

On Wed, Jun 22, 2016 at 7:55 PM, Liu, Yaxun (Sam)  wrote:
> The cmake of Cygwin itself does not support llvm. Which cmake did you use on 
> Cygwin? Thanks.

I use native windows cmake and this is the only test that ever failed with this 
setup. chmod is from native cygwin which seems to be the problem.

ismail
diff --git a/test/Headers/opencl-c-header.cl b/test/Headers/opencl-c-header.cl
index 4ba3b27..3723935 100644
--- a/test/Headers/opencl-c-header.cl
+++ b/test/Headers/opencl-c-header.cl
@@ -71,11 +71,11 @@
 // RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK 
--check-prefix=CHECK-MOD %s
 // RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck 
--check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
 // RUN: %clang_cc1 -triple amdgcn--amdhsa -emit-llvm -o - -cl-std=CL2.0  
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck 
--check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
-// RUN: chmod u-w %t/* 
+// RUN: chmod u-w %t 
 // RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK 
--check-prefix=CHECK-MOD %s
 // RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck 
--check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
 // RUN: %clang_cc1 -triple amdgcn--amdhsa -emit-llvm -o - -cl-std=CL2.0 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -ftime-report %s 2>&1 | FileCheck 
--check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
-// RUN: chmod u+w %t/*
+// RUN: chmod u+w %t
 
 char f(char x) {
 #if __OPENCL_C_VERSION__ != CL_VERSION_2_0
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

[Liu, Yaxun (Sam) via cfe-commits]

The returned pointer should point to the same pointee type as the argument. 
Header file cannot guarantee that.

Sam

-Original Message-
From: Jan Vesely [mailto:jan.ves...@rutgers.edu] 
Sent: Wednesday, June 22, 2016 10:20 PM
To: Liu, Yaxun (Sam) ; xiuli...@outlook.com; 
anastasia.stul...@arm.com
Cc: jan.ves...@rutgers.edu; Stellard, Thomas ; 
cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin 
functions.

jvesely added a subscriber: jvesely.
jvesely added a comment.

Why couldn't this be declared in CLC header? There are multiple instances of 
declaring functions for all available types in libclc (see for example convert 
functions in convert.h).


Repository:
  rL LLVM

http://reviews.llvm.org/D19932



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

RE: r274150 - [OpenCL] Allow -cl-std and other standard -cl- options in driver

[Liu, Yaxun (Sam) via cfe-commits]

We will take a look. Thanks.

Sam

-Original Message-
From: Benjamin Kramer [mailto:benny@gmail.com] 
Sent: Thursday, June 30, 2016 5:27 AM
To: Liu, Yaxun (Sam) ; Shi, Aaron (en ye) 
; anastasia.stul...@arm.com
Cc: cfe-commits 
Subject: Re: r274150 - [OpenCL] Allow -cl-std and other standard -cl- options 
in driver

The opencl.cl test never ran due to a lit misconfiguration (see r274221). Now 
that's fixed but the test doesn't pass, so I marked it as XFAIL. Can you take a 
look?

On Wed, Jun 29, 2016 at 9:39 PM, Yaxun Liu via cfe-commits 
 wrote:
> Author: yaxunl
> Date: Wed Jun 29 14:39:32 2016
> New Revision: 274150
>
> URL: http://llvm.org/viewvc/llvm-project?rev=274150&view=rev
> Log:
> [OpenCL] Allow -cl-std and other standard -cl- options in driver
>
> Allow -cl-std and other standard -cl- options from cc1 to driver.
>
> Added a test for the options moved.
>
> Patch by Aaron En Ye Shi.
>
> Differential Revision: http://reviews.llvm.org/D21031
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> cfe/trunk/include/clang/Driver/CC1Options.td
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/test/Driver/opencl.cl
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diag
> nosticFrontendKinds.td?rev=274150&r1=274149&r2=274150&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td 
> (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Wed Jun 
> +++ 29 14:39:32 2016
> @@ -214,4 +214,7 @@ def err_missing_vfs_overlay_file : Error
>"virtual filesystem overlay file '%0' not found">, DefaultFatal;  
> def err_invalid_vfs_overlay : Error<
>"invalid virtual filesystem overlay file '%0'">, DefaultFatal; -}
> +
> +def warn_option_invalid_ocl_version : Warning<
> +  "OpenCL version %0 does not support the option '%1'">, 
> +InGroup; }
> \ No newline at end of file
>
> Modified: cfe/trunk/include/clang/Driver/CC1Options.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1
> Options.td?rev=274150&r1=274149&r2=274150&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
> +++ cfe/trunk/include/clang/Driver/CC1Options.td Wed Jun 29 14:39:32 
> +++ 2016
> @@ -664,31 +664,6 @@ def detailed_preprocessing_record : Flag
>HelpText<"include a detailed record of preprocessing actions">;
>
>  
> //===-
> -===//
> -// OpenCL Options
> -//===
> --===//
> -
> -def cl_opt_disable : Flag<["-"], "cl-opt-disable">,
> -  HelpText<"OpenCL only. This option disables all optimizations. The 
> default is optimizations are enabled.">; -def cl_strict_aliasing : 
> Flag<["-"], "cl-strict-aliasing">,
> -  HelpText<"OpenCL only. This option does nothing and is for 
> compatibility with OpenCL 1.0">; -def cl_single_precision_constant : 
> Flag<["-"], "cl-single-precision-constant">,
> -  HelpText<"OpenCL only. Treat double precision floating-point 
> constant as single precision constant.">; -def cl_finite_math_only : 
> Flag<["-"], "cl-finite-math-only">,
> -  HelpText<"OpenCL only. Allow floating-point optimizations that 
> assume arguments and results are not NaNs or +-Inf.">; -def 
> cl_kernel_arg_info : Flag<["-"], "cl-kernel-arg-info">,
> -  HelpText<"OpenCL only. Generate kernel argument metadata.">; -def 
> cl_unsafe_math_optimizations : Flag<["-"], 
> "cl-unsafe-math-optimizations">,
> -  HelpText<"OpenCL only. Allow unsafe floating-point optimizations.  
> Also implies -cl-no-signed-zeros and -cl-mad-enable">; -def 
> cl_fast_relaxed_math : Flag<["-"], "cl-fast-relaxed-math">,
> -  HelpText<"OpenCL only. Sets -cl-finite-math-only and 
> -cl-unsafe-math-optimizations, and defines __FAST_RELAXED_MATH__">; 
> -def cl_mad_enable : Flag<["-"], "cl-mad-enable">,
> -  HelpText<"OpenCL only. Enable less precise MAD instructions to be 
> generated.">; -def cl_std_EQ : Joined<["-"], "cl-std=">,
> -  HelpText<"OpenCL language standard to compile for">; -def 
> cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">,
> -  HelpText<"OpenCL only. Allow denormals to be flushed to zero">;
> -
> -//===
> --===//
>  // CUDA Options
>  
> //===-
> -===//
>
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Opt
> ions.td?rev=274150&r1=274149&r2=274150&view=diff
> ===

RE: [PATCH] D21031: [OpenCL] Allow -cl-std and other standard -cl- options in driver

[Liu, Yaxun (Sam) via cfe-commits]

We will add it. Thanks.

Sam

-Original Message-
From: Jan Vesely [mailto:jan.ves...@rutgers.edu] 
Sent: Friday, July 1, 2016 4:59 PM
To: Shi, Aaron (en ye) ; anastasia.stul...@arm.com
Cc: jan.ves...@rutgers.edu; nhaus...@gmail.com; rich...@metafoo.co.uk; 
alexey.ba...@intel.com; xiuli...@outlook.com; cfe-commits@lists.llvm.org; Liu, 
Yaxun (Sam) 
Subject: Re: [PATCH] D21031: [OpenCL] Allow -cl-std and other standard -cl- 
options in driver

jvesely added a subscriber: jvesely.


Comment at: cfe/trunk/include/clang/Driver/Options.td:381
@@ +380,3 @@
+def cl_unsafe_math_optimizations : Flag<["-"], 
+"cl-unsafe-math-optimizations">, Group, 
+Flags<[CC1Option]>,
+  HelpText<"OpenCL only. Allow unsafe floating-point optimizations.  
+Also implies -cl-no-signed-zeros and -cl-mad-enable.">; def 
+cl_fast_relaxed_math : Flag<["-"], "cl-fast-relaxed-math">, 
+Group, Flags<[CC1Option]>,

It'd be nice if you added cl-no-signed-zeros since it's mentioned here


Repository:
  rL LLVM

http://reviews.llvm.org/D21031



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

RE: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

[Liu, Yaxun (Sam) via cfe-commits]

+ Brian

Hi Anastasia,

The advantage for translating sampler variable to a global variable with 
__sampler_initializer type is that it is consistent with OpenCL C++ and SPIRV, 
so it is easier to translate the IR to SPIRV.

About the type name of sampler_t in LLVM, using a name without `.` allows 
library functions to use the concrete sampler types directly without casting. 
Casting not only affects the readability of the library code, but also causes 
extra efforts in optimizing these codes.
 
Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Thursday, July 7, 2016 11:29 AM
To: Liu, Yaxun (Sam) ; anastasia.stul...@arm.com; 
alexey.ba...@intel.com; xiuli...@outlook.com
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and 
function call for the initializer

Anastasia added inline comments.


Comment at: include/clang/AST/BuiltinTypes.def:164
@@ +163,3 @@
+// Internal OpenCL sampler initializer type.
+BUILTIN_TYPE(OCLSamplerInit, OCLSamplerInitTy)
+

> However, we want it to be translated to __sampler_initializer* type.

This is precisely what I am not able to understand. Why do we want it to be  
__sampler_initializer* type? I don't think we want that. If it was a sampler 
type originally in OpenCL, it should be kept a sampler type in LLVM too.


Comment at: include/clang/AST/OperationKinds.def:328
@@ +327,3 @@
+// Convert an integer initializer to an OpenCL sampler initializer.
+CAST_OPERATION(IntToOCLSamplerInitializer)
+

> In codegen, we want to insert a call of __transform_sampler_initializer for 
> the reference of variable a, not the definition of variable a.
 I think the original discussion was to insert the call on the declaration of 
sampler variable as an initializer. This would allow us to use variable itself 
"as is" everywhere else.

So if let's say we compile:
  constant sampler_t a = 0;
  kernel void f() {
 g(a);
  }

the code we are generating would be equivalent to:

  kernel void f() {
__sampler* a = __transform_sampler_initializer(0);
g(a);
  }

Why are we not using this approach?

I think we discussed later that we could generate a struct of initializers 
instead of just int (0 -> struct sampler_initializer). Here is the description: 
http://lists.llvm.org/pipermail/cfe-dev/2016-June/049389.html
But general principle still was as described above...


Comment at: include/clang/Basic/DiagnosticGroups.td:876
@@ +875,3 @@
+// A warning group for warnings about code that clang accepts when // 
+compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
+def SpirCompat : DiagGroup<"spir-compat">;

I see. This is just because OpenCL doesn't allocate actual constants for 
different allowed sampler modes whereas SPIR does.


Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:52
@@ +51,3 @@
+   Ctx, "__sampler"),
+   CGM.getContext().getTargetAddressSpace(
+   LangAS::opencl_constant));

If you do conversion struct ptr <-> opaque ptr in this function, it should be 
fine? I would imagine images and other OpenCL types have the same requirement.


http://reviews.llvm.org/D21567



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