Re: r365499 - [OpenCL][Sema] Fix builtin rewriting

2019-07-10 Thread Reid Kleckner via cfe-commits 
NP, I forgot to follow up mentioning that I tweaked the test to fix the
failure in https://reviews.llvm.org/rC365557. It would be good to
eventually mangle whatever OpenCL thing is being mangled here in the MS
ABI, but that can be future work.

On Wed, Jul 10, 2019 at 2:45 AM Marco Antognini 
wrote:

> Hello Reid,
>
> Thanks for reporting it. I filed a bug to fix that (
> http://llvm.org/PR42560). However, I'm not sure I'll have time this week
> to look at it, especially given that we have no Windows builder around
> here. If you are critically blocked by this, could you check that adding
> '// UNSUPPORTED: system-windows' properly disables the test? And if so,
> feel free to commit for now.
>
> Marco
> --
> *From:* Reid Kleckner 
> *Sent:* 09 July 2019 21:47
> *To:* Marco Antognini
> *Cc:* cfe-commits
> *Subject:* Re: r365499 - [OpenCL][Sema] Fix builtin rewriting
>
> FYI, your test seems to fail on Windows:
> FAIL: Clang :: CodeGenOpenCL/pipe_builtin.cl (4679 of 15176)
>  TEST 'Clang :: CodeGenOpenCL/pipe_builtin.cl' FAILED
> 
> Script:
> --
> : 'RUN: at line 1';
> c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe -cc1
> -internal-isystem
> c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include
> -nostdsysteminc -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=c++ -o -
> C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
> pipe_builtin.cl |
> c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\filecheck.exe
> C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
> pipe_builtin.cl
> --
> Exit Code: 2
>
> Command Output (stdout):
> --
> $ ":" "RUN: at line 1"
> $ "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe"
> "-cc1" "-internal-isystem"
> "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include"
> "-nostdsysteminc" "-emit-llvm" "-cl-ext=+cl_khr_subgroups" "-O0"
> "-cl-std=c++" "-o" "-"
> "C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
> pipe_builtin.cl"
> # command stderr:
>
> C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl:9:1:
> error: cannot mangle this OpenCL pipe type yet
>
> void test1(read_only pipe int p, global int *ptr) {
>
> ^~~
>
> 1 error generated.
>
> On Tue, Jul 9, 2019 at 8:04 AM Marco Antognini via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: mantognini
> Date: Tue Jul  9 08:04:23 2019
> New Revision: 365499
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365499=rev
> Log:
> [OpenCL][Sema] Fix builtin rewriting
>
> This patch ensures built-in functions are rewritten using the proper
> parent declaration.
>
> Existing tests are modified to run in C++ mode to ensure the
> functionality works also with C++ for OpenCL while not increasing the
> testing runtime.
>
> Modified:
> cfe/trunk/include/clang/Basic/Builtins.def
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/test/CodeGenOpenCL/builtins.cl
> cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
> cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/include/clang/Basic/Builtins.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=365499=365498=365499=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
> +++ cfe/trunk/include/clang/Basic/Builtins.def Tue Jul  9 08:04:23 2019
> @@ -1478,6 +1478,7 @@ BUILTIN(__builtin_coro_begin, "v*v*", "n
>  BUILTIN(__builtin_coro_end, "bv*Ib", "n")
>  BUILTIN(__builtin_coro_suspend, "cIb", "n")
>  BUILTIN(__builtin_coro_param, "bv*v*", "n")
> +
>  // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
>  // We need the generic prototype, since the packet type could be anything.
>  LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
> @@ -1513,6 +1514,8 @@ LANGBUILTIN(get_kernel_max_sub_group_siz
>  LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "Ui.", "tn",
> OCLC20_LANG)
>
>  // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
> +// FIXME: Pointer parameters of OpenCL builtins should have their address
> space
> +// requirement defined.
>  LANGBUILTIN(to_global, "v*v*", "tn", OCLC20_LAN

Re: r365499 - [OpenCL][Sema] Fix builtin rewriting

2019-07-10 Thread Marco Antognini via cfe-commits 
Hello Reid,

Thanks for reporting it. I filed a bug to fix that (http://llvm.org/PR42560). 
However, I'm not sure I'll have time this week to look at it, especially given 
that we have no Windows builder around here. If you are critically blocked by 
this, could you check that adding '// UNSUPPORTED: system-windows' properly 
disables the test? And if so, feel free to commit for now.

Marco

From: Reid Kleckner 
Sent: 09 July 2019 21:47
To: Marco Antognini
Cc: cfe-commits
Subject: Re: r365499 - [OpenCL][Sema] Fix builtin rewriting

FYI, your test seems to fail on Windows:
FAIL: Clang :: CodeGenOpenCL/pipe_builtin.cl<http://pipe_builtin.cl> (4679 of 
15176)
 TEST 'Clang :: 
CodeGenOpenCL/pipe_builtin.cl<http://pipe_builtin.cl>' FAILED 

Script:
--
: 'RUN: at line 1';   
c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe -cc1 
-internal-isystem 
c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include 
-nostdsysteminc -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=c++ -o - 
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl<http://pipe_builtin.cl>
 | c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\filecheck.exe 
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl<http://pipe_builtin.cl>
--
Exit Code: 2

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include" 
"-nostdsysteminc" "-emit-llvm" "-cl-ext=+cl_khr_subgroups" "-O0" "-cl-std=c++" 
"-o" "-" 
"C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl<http://pipe_builtin.cl>"
# command stderr:
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl:9:1:
 error: cannot mangle this OpenCL pipe type yet

void test1(read_only pipe int p, global int *ptr) {

^~~

1 error generated.

On Tue, Jul 9, 2019 at 8:04 AM Marco Antognini via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: mantognini
Date: Tue Jul  9 08:04:23 2019
New Revision: 365499

URL: http://llvm.org/viewvc/llvm-project?rev=365499=rev
Log:
[OpenCL][Sema] Fix builtin rewriting

This patch ensures built-in functions are rewritten using the proper
parent declaration.

Existing tests are modified to run in C++ mode to ensure the
functionality works also with C++ for OpenCL while not increasing the
testing runtime.

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CodeGenOpenCL/builtins.cl<http://builtins.cl>
cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl<http://pipe_builtin.cl>
cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl<http://to_addr_builtin.cl>

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=365499=365498=365499=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Tue Jul  9 08:04:23 2019
@@ -1478,6 +1478,7 @@ BUILTIN(__builtin_coro_begin, "v*v*", "n
 BUILTIN(__builtin_coro_end, "bv*Ib", "n")
 BUILTIN(__builtin_coro_suspend, "cIb", "n")
 BUILTIN(__builtin_coro_param, "bv*v*", "n")
+
 // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
 // We need the generic prototype, since the packet type could be anything.
 LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
@@ -1513,6 +1514,8 @@ LANGBUILTIN(get_kernel_max_sub_group_siz
 LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "Ui.", "tn", OCLC20_LANG)

 // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+// FIXME: Pointer parameters of OpenCL builtins should have their address space
+// requirement defined.
 LANGBUILTIN(to_global, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_local, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_private, "v*v*", "tn", OCLC20_LANG)

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=365499=365498=365499=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul  9 08:04:23 2019
@@ -5360,7 +5360,7 @@ static bool checkArgsForPlaceholders(Sem
 ///

Re: r365499 - [OpenCL][Sema] Fix builtin rewriting

2019-07-09 Thread Reid Kleckner via cfe-commits 
FYI, your test seems to fail on Windows:
FAIL: Clang :: CodeGenOpenCL/pipe_builtin.cl (4679 of 15176)
 TEST 'Clang :: CodeGenOpenCL/pipe_builtin.cl' FAILED

Script:
--
: 'RUN: at line 1';
c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe -cc1
-internal-isystem
c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include
-nostdsysteminc -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=c++ -o -
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
pipe_builtin.cl |
c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\filecheck.exe
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
pipe_builtin.cl
--
Exit Code: 2

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.exe"
"-cc1" "-internal-isystem"
"c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\9.0.0\include"
"-nostdsysteminc" "-emit-llvm" "-cl-ext=+cl_khr_subgroups" "-O0"
"-cl-std=c++" "-o" "-"
"C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\
pipe_builtin.cl"
# command stderr:
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\CodeGenOpenCL\pipe_builtin.cl:9:1:
error: cannot mangle this OpenCL pipe type yet

void test1(read_only pipe int p, global int *ptr) {

^~~

1 error generated.

On Tue, Jul 9, 2019 at 8:04 AM Marco Antognini via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: mantognini
> Date: Tue Jul  9 08:04:23 2019
> New Revision: 365499
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365499=rev
> Log:
> [OpenCL][Sema] Fix builtin rewriting
>
> This patch ensures built-in functions are rewritten using the proper
> parent declaration.
>
> Existing tests are modified to run in C++ mode to ensure the
> functionality works also with C++ for OpenCL while not increasing the
> testing runtime.
>
> Modified:
> cfe/trunk/include/clang/Basic/Builtins.def
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/test/CodeGenOpenCL/builtins.cl
> cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
> cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/include/clang/Basic/Builtins.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=365499=365498=365499=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
> +++ cfe/trunk/include/clang/Basic/Builtins.def Tue Jul  9 08:04:23 2019
> @@ -1478,6 +1478,7 @@ BUILTIN(__builtin_coro_begin, "v*v*", "n
>  BUILTIN(__builtin_coro_end, "bv*Ib", "n")
>  BUILTIN(__builtin_coro_suspend, "cIb", "n")
>  BUILTIN(__builtin_coro_param, "bv*v*", "n")
> +
>  // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
>  // We need the generic prototype, since the packet type could be anything.
>  LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
> @@ -1513,6 +1514,8 @@ LANGBUILTIN(get_kernel_max_sub_group_siz
>  LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "Ui.", "tn",
> OCLC20_LANG)
>
>  // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
> +// FIXME: Pointer parameters of OpenCL builtins should have their address
> space
> +// requirement defined.
>  LANGBUILTIN(to_global, "v*v*", "tn", OCLC20_LANG)
>  LANGBUILTIN(to_local, "v*v*", "tn", OCLC20_LANG)
>  LANGBUILTIN(to_private, "v*v*", "tn", OCLC20_LANG)
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=365499=365498=365499=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul  9 08:04:23 2019
> @@ -5360,7 +5360,7 @@ static bool checkArgsForPlaceholders(Sem
>  ///  FunctionDecl is returned.
>  /// TODO: Handle pointer return types.
>  static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext
> ,
> -const FunctionDecl *FDecl,
> +FunctionDecl *FDecl,
>  MultiExprArg ArgExprs) {
>
>QualType DeclType = FDecl->getType();
> @@ -5408,7 +5408,7 @@ static FunctionDecl *rewriteBuiltinFunct
>FunctionProtoType::ExtProtoInfo EPI;
>QualType OverloadTy = Context.getFunctionType(FT->getReturnType(),
>  OverloadParams, EPI);
> -  DeclContext *Parent = Context.getTranslationUnitDecl();
> +  DeclContext *Parent = FDecl->getParent();
>FunctionDecl *OverloadDecl = FunctionDecl::Create(Context, Parent,
>  FDecl->getLocation(),
>  FDecl->getLocation(),
>
> Modified:

r365499 - [OpenCL][Sema] Fix builtin rewriting

2019-07-09 Thread Marco Antognini via cfe-commits 
Author: mantognini
Date: Tue Jul  9 08:04:23 2019
New Revision: 365499

URL: http://llvm.org/viewvc/llvm-project?rev=365499=rev
Log:
[OpenCL][Sema] Fix builtin rewriting

This patch ensures built-in functions are rewritten using the proper
parent declaration.

Existing tests are modified to run in C++ mode to ensure the
functionality works also with C++ for OpenCL while not increasing the
testing runtime.

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CodeGenOpenCL/builtins.cl
cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=365499=365498=365499=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Tue Jul  9 08:04:23 2019
@@ -1478,6 +1478,7 @@ BUILTIN(__builtin_coro_begin, "v*v*", "n
 BUILTIN(__builtin_coro_end, "bv*Ib", "n")
 BUILTIN(__builtin_coro_suspend, "cIb", "n")
 BUILTIN(__builtin_coro_param, "bv*v*", "n")
+
 // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
 // We need the generic prototype, since the packet type could be anything.
 LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
@@ -1513,6 +1514,8 @@ LANGBUILTIN(get_kernel_max_sub_group_siz
 LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "Ui.", "tn", OCLC20_LANG)
 
 // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+// FIXME: Pointer parameters of OpenCL builtins should have their address space
+// requirement defined.
 LANGBUILTIN(to_global, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_local, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_private, "v*v*", "tn", OCLC20_LANG)

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=365499=365498=365499=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul  9 08:04:23 2019
@@ -5360,7 +5360,7 @@ static bool checkArgsForPlaceholders(Sem
 ///  FunctionDecl is returned.
 /// TODO: Handle pointer return types.
 static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext 
,
-const FunctionDecl *FDecl,
+FunctionDecl *FDecl,
 MultiExprArg ArgExprs) {
 
   QualType DeclType = FDecl->getType();
@@ -5408,7 +5408,7 @@ static FunctionDecl *rewriteBuiltinFunct
   FunctionProtoType::ExtProtoInfo EPI;
   QualType OverloadTy = Context.getFunctionType(FT->getReturnType(),
 OverloadParams, EPI);
-  DeclContext *Parent = Context.getTranslationUnitDecl();
+  DeclContext *Parent = FDecl->getParent();
   FunctionDecl *OverloadDecl = FunctionDecl::Create(Context, Parent,
 FDecl->getLocation(),
 FDecl->getLocation(),

Modified: cfe/trunk/test/CodeGenOpenCL/builtins.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/builtins.cl?rev=365499=365498=365499=diff
==
--- cfe/trunk/test/CodeGenOpenCL/builtins.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/builtins.cl Tue Jul  9 08:04:23 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -finclude-default-header -cl-std=CL2.0 -O0 -emit-llvm -o 
- -triple "spir-unknown-unknown" | FileCheck %s
+// RUN: %clang_cc1 %s -finclude-default-header -cl-std=c++ -fblocks -O0 
-emit-llvm -o - -triple "spir-unknown-unknown" | FileCheck %s
 
 void testBranchingOnEnqueueKernel(queue_t default_queue, unsigned flags, 
ndrange_t ndrange) {
 // Ensure `enqueue_kernel` can be branched upon.

Modified: cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl?rev=365499=365498=365499=diff
==
--- cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl Tue Jul  9 08:04:23 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=CL2.0 -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=c++ -o - 
%s | FileCheck %s
 
 // CHECK-DAG: %opencl.pipe_ro_t = type opaque
 // CHECK-DAG: %opencl.pipe_wo_t = type opaque

Modified: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl?rev=365499=365498=365499=diff