[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D72857#1895649 , @bader wrote:

> In D72857#1895625 , @thakis wrote:
>
> > This landed here: 
> > https://github.com/llvm/llvm-project/commit/bd97704eb5a95ecb048ce343c1a4be5d94e5
> >
> > It broke tests on mac: http://45.33.8.238/mac/9011/step_7.txt
> >
> > Please take a look, and if it takes a while please revert while you 
> > investigate.
>
>
> @thakis, thank for letting me know. I've reverted it at 
> 740ed617f7d4d16e7883636c5eff994f8be7eba4 
> . Sorry 
> for inconvenience.


@thakis, could you help to find the logs for the "build" step?
I don't have access to a Mac system and I can't reproduce the problem on my 
Linux system, but just looking at the log it looks like that changes to 
clang/include/clang/Driver/Options.td were not applied.

  :7:42: note: possible intended match here
  clang: warning: argument unused during compilation: '-fsycl' 
[-Wunused-command-line-argument]

I'd like to make sure that clang is built with the patch applied.

I also noticed that on your system uses gn instead of cmake. Can it be that 
there is a missing dependency in GN files, which lead to skipping clang 
re-build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd97704e: [SYCL] Driver option to select SYCL version 
(authored by Ruyk, committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Frontend/sycl-aux-triple.cpp
  clang/test/Preprocessor/sycl-macro.cpp
  clang/test/SemaSYCL/kernel-attribute.cpp

Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- clang/test/SemaSYCL/kernel-attribute.cpp
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl -fsycl-is-device -verify %s
 
 // Only function templates
 [[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2017 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Frontend/sycl-aux-triple.cpp
===
--- clang/test/Frontend/sycl-aux-triple.cpp
+++ clang/test/Frontend/sycl-aux-triple.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
 
 // CHECK-NOT:#define __x86_64__ 1
 // CHECK-SYCL:#define __x86_64__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -460,6 +460,13 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  if (LangOpts.SYCL) {
+// SYCL Version is set to a value when building SYCL applications
+if (LangOpts.SYCLVersion == 2017)
+  Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D72857#1895625 , @thakis wrote:

> This landed here: 
> https://github.com/llvm/llvm-project/commit/bd97704eb5a95ecb048ce343c1a4be5d94e5
>
> It broke tests on mac: http://45.33.8.238/mac/9011/step_7.txt
>
> Please take a look, and if it takes a while please revert while you 
> investigate.


@thakis, thank for letting me know. I've reverted it at 
740ed617f7d4d16e7883636c5eff994f8be7eba4 
. Sorry 
for inconvenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This landed here: 
https://github.com/llvm/llvm-project/commit/bd97704eb5a95ecb048ce343c1a4be5d94e5

It broke tests on mac: http://45.33.8.238/mac/9011/step_7.txt

Please take a look, and if it takes a while please revert while you investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

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

If no other comments, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 246072.
bader added a comment.

Rebase.

Any other comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Frontend/sycl-aux-triple.cpp
  clang/test/Preprocessor/sycl-macro.cpp
  clang/test/SemaSYCL/kernel-attribute.cpp

Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- clang/test/SemaSYCL/kernel-attribute.cpp
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl -fsycl-is-device -verify %s
 
 // Only function templates
 [[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2017 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Frontend/sycl-aux-triple.cpp
===
--- clang/test/Frontend/sycl-aux-triple.cpp
+++ clang/test/Frontend/sycl-aux-triple.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
 
 // CHECK-NOT:#define __x86_64__ 1
 // CHECK-SYCL:#define __x86_64__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -460,6 +460,13 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  if (LangOpts.SYCL) {
+// SYCL Version is set to a value when building SYCL applications
+if (LangOpts.SYCLVersion == 2017)
+  Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:3419
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"2015, 121, 
1.2.1, sycl-1.2.1">;
 

keryell wrote:
> I suggest replacing all the 2015 by 2017.
> While this is true SYCL 1.2 was published in 2015, SYCL 1.2.1 was published 
> in 2017. Only 1.2.1 matters here since 1.2 was never fully implemented by any 
> conformant implementation. https://en.wikipedia.org/wiki/SYCL
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 245373.
bader marked 12 inline comments as done.
bader added a comment.

Rebase to ToT and address comments from Ronan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Frontend/sycl-aux-triple.cpp
  clang/test/Preprocessor/sycl-macro.cpp
  clang/test/SemaSYCL/kernel-attribute.cpp

Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- clang/test/SemaSYCL/kernel-attribute.cpp
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl -fsycl-is-device -verify %s
 
 // Only function templates
 [[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2017 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Frontend/sycl-aux-triple.cpp
===
--- clang/test/Frontend/sycl-aux-triple.cpp
+++ clang/test/Frontend/sycl-aux-triple.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
 
 // CHECK-NOT:#define __x86_64__ 1
 // CHECK-SYCL:#define __x86_64__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2017 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -460,6 +460,13 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  if (LangOpts.SYCL) {
+// SYCL Version is set to a value when building SYCL applications
+if (LangOpts.SYCLVersion == 2017)
+  Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Ronan Keryell via Phabricator via cfe-commits]

keryell added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3419
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"2015, 121, 
1.2.1, sycl-1.2.1">;
 

I suggest replacing all the 2015 by 2017.
While this is true SYCL 1.2 was published in 2015, SYCL 1.2.1 was published in 
2017. Only 1.2.1 matters here since 1.2 was never fully implemented by any 
conformant implementation. https://en.wikipedia.org/wiki/SYCL



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4034
+  // Ensure the default version in SYCL mode is 1.2.1 (aka 2015)
+  CmdArgs.push_back("-sycl-std=2015");
+}

Replace 2015 by 2017 in both lines above.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2554
+  Opts.SYCLVersion = llvm::StringSwitch(A->getValue())
+ .Cases("2015", "1.2.1", "121", "sycl-1.2.1", 2015)
+ .Default(0U);

Replace 2015 by 2017.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:456
+// SYCL Version is set to a value when building SYCL applications
+if (LangOpts.SYCLVersion == 2015)
+  Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");

Replace 2015 by 2017.



Comment at: clang/test/Driver/sycl.c:5
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s 
--check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s 
--check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s 
--check-prefix=ENABLED

Replace all the 2015 by 2017 here and below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 245213.
bader added a comment.

Address comments from Victor and Alexey.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Frontend/sycl-aux-triple.cpp
  clang/test/Preprocessor/sycl-macro.cpp
  clang/test/SemaSYCL/kernel-attribute.cpp

Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- clang/test/SemaSYCL/kernel-attribute.cpp
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl -fsycl-is-device -verify %s
 
 // Only function templates
 [[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Frontend/sycl-aux-triple.cpp
===
--- clang/test/Frontend/sycl-aux-triple.cpp
+++ clang/test/Frontend/sycl-aux-triple.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
 
 // CHECK-NOT:#define __x86_64__ 1
 // CHECK-SYCL:#define __x86_64__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,13 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  if (LangOpts.SYCL) {
+// SYCL Version is set to a value when building SYCL applications
+if (LangOpts.SYCLVersion == 2015)
+  Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2549
+  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {
+// -sycl-std applies to any SYCL source, not only those containing kernels,

Just `Opt.SYCL` should be enough



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:455
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:

Must be guarded with `LangOpts.SYCL`:
```
if (LangOpts.SYCL) ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader marked 3 inline comments as done.
bader added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:206
 LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "C++ for OpenCL version")
+ENUM_LANGOPT(SYCLVersion, SYCLVersionList, 4, SYCLVersionList::undefined, 
"Version of the SYCL standard used")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")

Naghasan wrote:
> Ruyk wrote:
> > bader wrote:
> > > All other language options controlling standard versions are added as 
> > > "LANGOPT" i.e. `int`. Why SYCLVersion is different?
> > > @Ruyk, do you think we should convert other options (e.g. `OpenCL`) to 
> > > enums as well?
> > Thats a good point. I don't see strong reasons for the enum, basically I 
> > read the comment in 
> > https://code.woboq.org/llvm/clang/include/clang/Basic/LangOptions.def.html#22
> > 
> > 
> > ```
> > // ENUM_LANGOPT: for options that have enumeration, rather than unsigned, 
> > type.
> > ```
> > 
> > And since there is a known set of SYCL specifications, made more sense to 
> > enumerate it.
> > It also simplifies writing variants to 1.2.1 (e.g. 1.2.1-oneapi) in the 
> > code since then you can add another entry to the enum.
> > 
> > But no strong feelings, so feel free to change it.
> > 
> `int` allows the use of relational operators which should ease version 
> managements.
> 
> As `SYCLVersionList` is a strongly typed enum so this may not be the best, 
> and as the SYCL version are now meant to be a year `int` should do just fine.
Okay. I'll change the type from enum to int.



Comment at: clang/include/clang/Driver/Options.td:3401
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"1.2.1">;
 

Naghasan wrote:
> Ruyk wrote:
> > bader wrote:
> > > What do you think we integrate sycl versions to existing clang options 
> > > controlling language version: `-std`.
> > > As far as I can see it's used for all the C/C+ extensions like 
> > > OpenMP/OpenCL/CUDA/HIP/ObjC.
> > > 
> > > If I understand correctly clang supports `-cl-std` only because it's 
> > > required by OpenCL standard. Similar option (i.e. `-sycl-std`) is not 
> > > required by the SYCL specification, so using `-std` is more aligned with 
> > > existing clang design.
> > > 
> > > See clang/include/clang/Basic/LangStandard.h and 
> > > clang/include/clang/Basic/LangStandards.def.
> > In the case of SYCL, you may want to compile your code with C++17 and SYCL 
> > 2015, in which case you need both -std=c++17 and -sycl=sycl-2015 . 
> > SYCL specification mandates a minimum C++ version but users can write code 
> > on newer versions as long as the code in the kernel scope is still valid.
> +1 on this, ComputeCpp used to mix-up both and this proved to be complex to 
> manage. It also integrates better with build systems.
Okay. I'll leave it as a separate option.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

Naghasan wrote:
> bader wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > ABataev wrote:
> > > > > bader wrote:
> > > > > > ABataev wrote:
> > > > > > > bader wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > bader wrote:
> > > > > > > > > > ABataev wrote:
> > > > > > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > > > > > ```
> > > > > > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > Does it really has to? This logic is already present in the 
> > > > > > > > > > driver and it makes front-end tests verbose `%clang_cc1 
> > > > > > > > > > -fsycl -fsycl-is-device`.
> > > > > > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > > > > > Looking how CUDA/OpenMP options are handled, not all of 
> > > > > > > > > > them are processed using this pattern.
> > > > > > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > > > > > differently, because it has its own kind of files (.cu) and 
> > > > > > > > > Cuda is triggered by the language switch (-x cu). Seems to 
> > > > > > > > > me, you're using something close to OpenMP model, no? Or do 
> > > > > > > > > you want to define your own language kind just like Cuda?
> > > > > > > > I applied you suggest, although I don't fully understand the 
> > > > > > > > need of using two options instead of two. I would prefer having 
> > > > > > > > following code:
> > > > > > > > ```
> > > > > > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || 
> > > > > > > > Opts.SYCLIsDevice; // -fsycl-is-device enable SYCL 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:206
 LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "C++ for OpenCL version")
+ENUM_LANGOPT(SYCLVersion, SYCLVersionList, 4, SYCLVersionList::undefined, 
"Version of the SYCL standard used")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")

Ruyk wrote:
> bader wrote:
> > All other language options controlling standard versions are added as 
> > "LANGOPT" i.e. `int`. Why SYCLVersion is different?
> > @Ruyk, do you think we should convert other options (e.g. `OpenCL`) to 
> > enums as well?
> Thats a good point. I don't see strong reasons for the enum, basically I read 
> the comment in 
> https://code.woboq.org/llvm/clang/include/clang/Basic/LangOptions.def.html#22
> 
> 
> ```
> // ENUM_LANGOPT: for options that have enumeration, rather than unsigned, 
> type.
> ```
> 
> And since there is a known set of SYCL specifications, made more sense to 
> enumerate it.
> It also simplifies writing variants to 1.2.1 (e.g. 1.2.1-oneapi) in the code 
> since then you can add another entry to the enum.
> 
> But no strong feelings, so feel free to change it.
> 
`int` allows the use of relational operators which should ease version 
managements.

As `SYCLVersionList` is a strongly typed enum so this may not be the best, and 
as the SYCL version are now meant to be a year `int` should do just fine.



Comment at: clang/include/clang/Driver/Options.td:3401
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"1.2.1">;
 

Ruyk wrote:
> bader wrote:
> > What do you think we integrate sycl versions to existing clang options 
> > controlling language version: `-std`.
> > As far as I can see it's used for all the C/C+ extensions like 
> > OpenMP/OpenCL/CUDA/HIP/ObjC.
> > 
> > If I understand correctly clang supports `-cl-std` only because it's 
> > required by OpenCL standard. Similar option (i.e. `-sycl-std`) is not 
> > required by the SYCL specification, so using `-std` is more aligned with 
> > existing clang design.
> > 
> > See clang/include/clang/Basic/LangStandard.h and 
> > clang/include/clang/Basic/LangStandards.def.
> In the case of SYCL, you may want to compile your code with C++17 and SYCL 
> 2015, in which case you need both -std=c++17 and -sycl=sycl-2015 . 
> SYCL specification mandates a minimum C++ version but users can write code on 
> newer versions as long as the code in the kernel scope is still valid.
+1 on this, ComputeCpp used to mix-up both and this proved to be complex to 
manage. It also integrates better with build systems.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > bader wrote:
> > > > > ABataev wrote:
> > > > > > bader wrote:
> > > > > > > ABataev wrote:
> > > > > > > > bader wrote:
> > > > > > > > > ABataev wrote:
> > > > > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > > > > ```
> > > > > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > Does it really has to? This logic is already present in the 
> > > > > > > > > driver and it makes front-end tests verbose `%clang_cc1 
> > > > > > > > > -fsycl -fsycl-is-device`.
> > > > > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > > > > Looking how CUDA/OpenMP options are handled, not all of them 
> > > > > > > > > are processed using this pattern.
> > > > > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > > > > differently, because it has its own kind of files (.cu) and 
> > > > > > > > Cuda is triggered by the language switch (-x cu). Seems to me, 
> > > > > > > > you're using something close to OpenMP model, no? Or do you 
> > > > > > > > want to define your own language kind just like Cuda?
> > > > > > > I applied you suggest, although I don't fully understand the need 
> > > > > > > of using two options instead of two. I would prefer having 
> > > > > > > following code:
> > > > > > > ```
> > > > > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; 
> > > > > > > // -fsycl-is-device enable SYCL mode as well
> > > > > > > ```
> > > > > > I'm not quite familiar with SYCL model, maybe this the right 
> > > > > > approach. You'd better try to provide more details. Are there any 
> > > > > > differences between just SYCL and SYCL-device compilation modes? 
> > > > > > How do you see the compilation sequence in general? At first 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Ruyman via Phabricator via cfe-commits]

Ruyk added a subscriber: Naghasan.
Ruyk added a comment.

@Naghasan please take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > ABataev wrote:
> > > > > bader wrote:
> > > > > > ABataev wrote:
> > > > > > > bader wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > > > ```
> > > > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > Does it really has to? This logic is already present in the 
> > > > > > > > driver and it makes front-end tests verbose `%clang_cc1 -fsycl 
> > > > > > > > -fsycl-is-device`.
> > > > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > > > Looking how CUDA/OpenMP options are handled, not all of them 
> > > > > > > > are processed using this pattern.
> > > > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > > > differently, because it has its own kind of files (.cu) and Cuda 
> > > > > > > is triggered by the language switch (-x cu). Seems to me, you're 
> > > > > > > using something close to OpenMP model, no? Or do you want to 
> > > > > > > define your own language kind just like Cuda?
> > > > > > I applied you suggest, although I don't fully understand the need 
> > > > > > of using two options instead of two. I would prefer having 
> > > > > > following code:
> > > > > > ```
> > > > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; 
> > > > > > // -fsycl-is-device enable SYCL mode as well
> > > > > > ```
> > > > > I'm not quite familiar with SYCL model, maybe this the right 
> > > > > approach. You'd better try to provide more details. Are there any 
> > > > > differences between just SYCL and SYCL-device compilation modes? How 
> > > > > do you see the compilation sequence in general? At first you're going 
> > > > > to compile the host version of the code, then the device? OR 
> > > > > something different?
> > > > I think SYCL model is quite similar to OpenMP model. One significant 
> > > > difference might be that to implement standard SYCL functionality we 
> > > > don't need any modifications for the host compiler. AFAIK, OpenMP 
> > > > compiler has to support OpenMP pragmas. 
> > > > We have a few attributes for Intel FPGA devices, which we can't 
> > > > pre-process with `__SYCL_DEVICE_ONLY__` macro and we have added 
> > > > "SYCL-host" mode to suppress compiler warnings about attributes ignored 
> > > > on the host. I think there might be other options how this can be 
> > > > achieved w/o adding new compilation mode and use regular C++ front-end 
> > > > as SYCL host compiler.
> > > > I think there is a difference between SYCL and SYCL-device modes, but 
> > > > it's mostly changes the compilation workflow in the driver, but not in 
> > > > the front-end. In SYCL-device mode, driver invokes only one front-end 
> > > > instance to generate offload code. In SYCL mode, driver invokes 
> > > > multiple front-end instances: one in SYCL-device mode and one in 
> > > > regular C++ mode (to be accurate we use SYCL-host mode, but as I 
> > > > mentioned above I don't think it really needed).
> > > > I hope it makes it clear now. Let me know if you have any other 
> > > > questions.
> > > > 
> > > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar 
> > > > to SYCL-device mode?
> > > > If so, can we assume that OpenMPIsDevice implies that OpenMP option is 
> > > > also set (implicitly)?
> > > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar 
> > > > to SYCL-device mode?
> > > 
> > > Well, for driver you need to pass `-fopenmp 
> > > -fopenmp-target=` to enable the compilation with 
> > > offloading support. In the frontend the host part is compiled with 
> > > `-fopenmp` only (+ aux-triple, probably), for devices - `-fopenmp 
> > > -fopenmp-is-device`. Without `-fopenmp` `-fopenmp-is-device` is just 
> > > ignored.
> > What is the reason to require the driver to pass both options for the 
> > devices? It sounds like `-fopenmp-is-device` should be enough to 
> > differentiate from the host part (`-fopenmp` only). Right?
> We treat a little bit differently. `-fopenmp` turns support for OpenMP, while 
> `-fopenmp-is-device` turns 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > bader wrote:
> > > > > ABataev wrote:
> > > > > > bader wrote:
> > > > > > > ABataev wrote:
> > > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > > ```
> > > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > > 
> > > > > > > > ```
> > > > > > > Does it really has to? This logic is already present in the 
> > > > > > > driver and it makes front-end tests verbose `%clang_cc1 -fsycl 
> > > > > > > -fsycl-is-device`.
> > > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > > Looking how CUDA/OpenMP options are handled, not all of them are 
> > > > > > > processed using this pattern.
> > > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > > differently, because it has its own kind of files (.cu) and Cuda is 
> > > > > > triggered by the language switch (-x cu). Seems to me, you're using 
> > > > > > something close to OpenMP model, no? Or do you want to define your 
> > > > > > own language kind just like Cuda?
> > > > > I applied you suggest, although I don't fully understand the need of 
> > > > > using two options instead of two. I would prefer having following 
> > > > > code:
> > > > > ```
> > > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
> > > > > -fsycl-is-device enable SYCL mode as well
> > > > > ```
> > > > I'm not quite familiar with SYCL model, maybe this the right approach. 
> > > > You'd better try to provide more details. Are there any differences 
> > > > between just SYCL and SYCL-device compilation modes? How do you see the 
> > > > compilation sequence in general? At first you're going to compile the 
> > > > host version of the code, then the device? OR something different?
> > > I think SYCL model is quite similar to OpenMP model. One significant 
> > > difference might be that to implement standard SYCL functionality we 
> > > don't need any modifications for the host compiler. AFAIK, OpenMP 
> > > compiler has to support OpenMP pragmas. 
> > > We have a few attributes for Intel FPGA devices, which we can't 
> > > pre-process with `__SYCL_DEVICE_ONLY__` macro and we have added 
> > > "SYCL-host" mode to suppress compiler warnings about attributes ignored 
> > > on the host. I think there might be other options how this can be 
> > > achieved w/o adding new compilation mode and use regular C++ front-end as 
> > > SYCL host compiler.
> > > I think there is a difference between SYCL and SYCL-device modes, but 
> > > it's mostly changes the compilation workflow in the driver, but not in 
> > > the front-end. In SYCL-device mode, driver invokes only one front-end 
> > > instance to generate offload code. In SYCL mode, driver invokes multiple 
> > > front-end instances: one in SYCL-device mode and one in regular C++ mode 
> > > (to be accurate we use SYCL-host mode, but as I mentioned above I don't 
> > > think it really needed).
> > > I hope it makes it clear now. Let me know if you have any other questions.
> > > 
> > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to 
> > > SYCL-device mode?
> > > If so, can we assume that OpenMPIsDevice implies that OpenMP option is 
> > > also set (implicitly)?
> > > Do I understand it correctly that OpenMP option enables OpenMP mode, 
> > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to 
> > > SYCL-device mode?
> > 
> > Well, for driver you need to pass `-fopenmp 
> > -fopenmp-target=` to enable the compilation with 
> > offloading support. In the frontend the host part is compiled with 
> > `-fopenmp` only (+ aux-triple, probably), for devices - `-fopenmp 
> > -fopenmp-is-device`. Without `-fopenmp` `-fopenmp-is-device` is just 
> > ignored.
> What is the reason to require the driver to pass both options for the 
> devices? It sounds like `-fopenmp-is-device` should be enough to 
> differentiate from the host part (`-fopenmp` only). Right?
We treat a little bit differently. `-fopenmp` turns support for OpenMP, while 
`-fopenmp-is-device` turns processing for device compilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



___
cfe-commits 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > ABataev wrote:
> > > > > bader wrote:
> > > > > > ABataev wrote:
> > > > > > > This option also must be controlled by `-fsycl`:
> > > > > > > ```
> > > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > > 
> > > > > > > ```
> > > > > > Does it really has to? This logic is already present in the driver 
> > > > > > and it makes front-end tests verbose `%clang_cc1 -fsycl 
> > > > > > -fsycl-is-device`.
> > > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > > Looking how CUDA/OpenMP options are handled, not all of them are 
> > > > > > processed using this pattern.
> > > > > In general, this is how we handle it in OpenMP. Cuda works 
> > > > > differently, because it has its own kind of files (.cu) and Cuda is 
> > > > > triggered by the language switch (-x cu). Seems to me, you're using 
> > > > > something close to OpenMP model, no? Or do you want to define your 
> > > > > own language kind just like Cuda?
> > > > I applied you suggest, although I don't fully understand the need of 
> > > > using two options instead of two. I would prefer having following code:
> > > > ```
> > > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
> > > > -fsycl-is-device enable SYCL mode as well
> > > > ```
> > > I'm not quite familiar with SYCL model, maybe this the right approach. 
> > > You'd better try to provide more details. Are there any differences 
> > > between just SYCL and SYCL-device compilation modes? How do you see the 
> > > compilation sequence in general? At first you're going to compile the 
> > > host version of the code, then the device? OR something different?
> > I think SYCL model is quite similar to OpenMP model. One significant 
> > difference might be that to implement standard SYCL functionality we don't 
> > need any modifications for the host compiler. AFAIK, OpenMP compiler has to 
> > support OpenMP pragmas. 
> > We have a few attributes for Intel FPGA devices, which we can't pre-process 
> > with `__SYCL_DEVICE_ONLY__` macro and we have added "SYCL-host" mode to 
> > suppress compiler warnings about attributes ignored on the host. I think 
> > there might be other options how this can be achieved w/o adding new 
> > compilation mode and use regular C++ front-end as SYCL host compiler.
> > I think there is a difference between SYCL and SYCL-device modes, but it's 
> > mostly changes the compilation workflow in the driver, but not in the 
> > front-end. In SYCL-device mode, driver invokes only one front-end instance 
> > to generate offload code. In SYCL mode, driver invokes multiple front-end 
> > instances: one in SYCL-device mode and one in regular C++ mode (to be 
> > accurate we use SYCL-host mode, but as I mentioned above I don't think it 
> > really needed).
> > I hope it makes it clear now. Let me know if you have any other questions.
> > 
> > Do I understand it correctly that OpenMP option enables OpenMP mode, which 
> > is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to 
> > SYCL-device mode?
> > If so, can we assume that OpenMPIsDevice implies that OpenMP option is also 
> > set (implicitly)?
> > Do I understand it correctly that OpenMP option enables OpenMP mode, which 
> > is equivalent of "SYCL-host" mode and enabling both OpenMP and 
> > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to 
> > SYCL-device mode?
> 
> Well, for driver you need to pass `-fopenmp 
> -fopenmp-target=` to enable the compilation with offloading 
> support. In the frontend the host part is compiled with `-fopenmp` only (+ 
> aux-triple, probably), for devices - `-fopenmp -fopenmp-is-device`. Without 
> `-fopenmp` `-fopenmp-is-device` is just ignored.
What is the reason to require the driver to pass both options for the devices? 
It sounds like `-fopenmp-is-device` should be enough to differentiate from the 
host part (`-fopenmp` only). Right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > bader wrote:
> > > > > ABataev wrote:
> > > > > > This option also must be controlled by `-fsycl`:
> > > > > > ```
> > > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > > 
> > > > > > ```
> > > > > Does it really has to? This logic is already present in the driver 
> > > > > and it makes front-end tests verbose `%clang_cc1 -fsycl 
> > > > > -fsycl-is-device`.
> > > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > > Looking how CUDA/OpenMP options are handled, not all of them are 
> > > > > processed using this pattern.
> > > > In general, this is how we handle it in OpenMP. Cuda works differently, 
> > > > because it has its own kind of files (.cu) and Cuda is triggered by the 
> > > > language switch (-x cu). Seems to me, you're using something close to 
> > > > OpenMP model, no? Or do you want to define your own language kind just 
> > > > like Cuda?
> > > I applied you suggest, although I don't fully understand the need of 
> > > using two options instead of two. I would prefer having following code:
> > > ```
> > > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
> > > -fsycl-is-device enable SYCL mode as well
> > > ```
> > I'm not quite familiar with SYCL model, maybe this the right approach. 
> > You'd better try to provide more details. Are there any differences between 
> > just SYCL and SYCL-device compilation modes? How do you see the compilation 
> > sequence in general? At first you're going to compile the host version of 
> > the code, then the device? OR something different?
> I think SYCL model is quite similar to OpenMP model. One significant 
> difference might be that to implement standard SYCL functionality we don't 
> need any modifications for the host compiler. AFAIK, OpenMP compiler has to 
> support OpenMP pragmas. 
> We have a few attributes for Intel FPGA devices, which we can't pre-process 
> with `__SYCL_DEVICE_ONLY__` macro and we have added "SYCL-host" mode to 
> suppress compiler warnings about attributes ignored on the host. I think 
> there might be other options how this can be achieved w/o adding new 
> compilation mode and use regular C++ front-end as SYCL host compiler.
> I think there is a difference between SYCL and SYCL-device modes, but it's 
> mostly changes the compilation workflow in the driver, but not in the 
> front-end. In SYCL-device mode, driver invokes only one front-end instance to 
> generate offload code. In SYCL mode, driver invokes multiple front-end 
> instances: one in SYCL-device mode and one in regular C++ mode (to be 
> accurate we use SYCL-host mode, but as I mentioned above I don't think it 
> really needed).
> I hope it makes it clear now. Let me know if you have any other questions.
> 
> Do I understand it correctly that OpenMP option enables OpenMP mode, which is 
> equivalent of "SYCL-host" mode and enabling both OpenMP and OpenMPIsDevice is 
> required for enabling OpenMP mode, which is similar to SYCL-device mode?
> If so, can we assume that OpenMPIsDevice implies that OpenMP option is also 
> set (implicitly)?
> Do I understand it correctly that OpenMP option enables OpenMP mode, which is 
> equivalent of "SYCL-host" mode and enabling both OpenMP and OpenMPIsDevice is 
> required for enabling OpenMP mode, which is similar to SYCL-device mode?

Well, for driver you need to pass `-fopenmp -fopenmp-target=` 
to enable the compilation with offloading support. In the frontend the host 
part is compiled with `-fopenmp` only (+ aux-triple, probably), for devices - 
`-fopenmp -fopenmp-is-device`. Without `-fopenmp` `-fopenmp-is-device` is just 
ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > ABataev wrote:
> > > > > This option also must be controlled by `-fsycl`:
> > > > > ```
> > > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > > 
> > > > > ```
> > > > Does it really has to? This logic is already present in the driver and 
> > > > it makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> > > > Can `-fsycl-is-device` imply `-fsycl`?
> > > > Looking how CUDA/OpenMP options are handled, not all of them are 
> > > > processed using this pattern.
> > > In general, this is how we handle it in OpenMP. Cuda works differently, 
> > > because it has its own kind of files (.cu) and Cuda is triggered by the 
> > > language switch (-x cu). Seems to me, you're using something close to 
> > > OpenMP model, no? Or do you want to define your own language kind just 
> > > like Cuda?
> > I applied you suggest, although I don't fully understand the need of using 
> > two options instead of two. I would prefer having following code:
> > ```
> > Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> > Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
> > -fsycl-is-device enable SYCL mode as well
> > ```
> I'm not quite familiar with SYCL model, maybe this the right approach. You'd 
> better try to provide more details. Are there any differences between just 
> SYCL and SYCL-device compilation modes? How do you see the compilation 
> sequence in general? At first you're going to compile the host version of the 
> code, then the device? OR something different?
I think SYCL model is quite similar to OpenMP model. One significant difference 
might be that to implement standard SYCL functionality we don't need any 
modifications for the host compiler. AFAIK, OpenMP compiler has to support 
OpenMP pragmas. 
We have a few attributes for Intel FPGA devices, which we can't pre-process 
with `__SYCL_DEVICE_ONLY__` macro and we have added "SYCL-host" mode to 
suppress compiler warnings about attributes ignored on the host. I think there 
might be other options how this can be achieved w/o adding new compilation mode 
and use regular C++ front-end as SYCL host compiler.
I think there is a difference between SYCL and SYCL-device modes, but it's 
mostly changes the compilation workflow in the driver, but not in the 
front-end. In SYCL-device mode, driver invokes only one front-end instance to 
generate offload code. In SYCL mode, driver invokes multiple front-end 
instances: one in SYCL-device mode and one in regular C++ mode (to be accurate 
we use SYCL-host mode, but as I mentioned above I don't think it really needed).
I hope it makes it clear now. Let me know if you have any other questions.

Do I understand it correctly that OpenMP option enables OpenMP mode, which is 
equivalent of "SYCL-host" mode and enabling both OpenMP and OpenMPIsDevice is 
required for enabling OpenMP mode, which is similar to SYCL-device mode?
If so, can we assume that OpenMPIsDevice implies that OpenMP option is also set 
(implicitly)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > This option also must be controlled by `-fsycl`:
> > > > ```
> > > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > > Args.hasArg(options::OPT_fsycl_is_device);
> > > > 
> > > > ```
> > > Does it really has to? This logic is already present in the driver and it 
> > > makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> > > Can `-fsycl-is-device` imply `-fsycl`?
> > > Looking how CUDA/OpenMP options are handled, not all of them are 
> > > processed using this pattern.
> > In general, this is how we handle it in OpenMP. Cuda works differently, 
> > because it has its own kind of files (.cu) and Cuda is triggered by the 
> > language switch (-x cu). Seems to me, you're using something close to 
> > OpenMP model, no? Or do you want to define your own language kind just like 
> > Cuda?
> I applied you suggest, although I don't fully understand the need of using 
> two options instead of two. I would prefer having following code:
> ```
> Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
> -fsycl-is-device enable SYCL mode as well
> ```
I'm not quite familiar with SYCL model, maybe this the right approach. You'd 
better try to provide more details. Are there any differences between just SYCL 
and SYCL-device compilation modes? How do you see the compilation sequence in 
general? At first you're going to compile the host version of the code, then 
the device? OR something different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader marked 3 inline comments as done.
bader added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > This option also must be controlled by `-fsycl`:
> > > ```
> > > Opts.SYCLIsDevice =  Opts.SYCL && 
> > > Args.hasArg(options::OPT_fsycl_is_device);
> > > 
> > > ```
> > Does it really has to? This logic is already present in the driver and it 
> > makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> > Can `-fsycl-is-device` imply `-fsycl`?
> > Looking how CUDA/OpenMP options are handled, not all of them are processed 
> > using this pattern.
> In general, this is how we handle it in OpenMP. Cuda works differently, 
> because it has its own kind of files (.cu) and Cuda is triggered by the 
> language switch (-x cu). Seems to me, you're using something close to OpenMP 
> model, no? Or do you want to define your own language kind just like Cuda?
I applied you suggest, although I don't fully understand the need of using two 
options instead of two. I would prefer having following code:
```
Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // 
-fsycl-is-device enable SYCL mode as well
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 243856.
bader marked 3 inline comments as done.
bader added a comment.

Applied review commits from Alexey.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Frontend/sycl-aux-triple.cpp
  clang/test/Preprocessor/sycl-macro.cpp
  clang/test/SemaSYCL/kernel-attribute.cpp

Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- clang/test/SemaSYCL/kernel-attribute.cpp
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl -fsycl-is-device -verify %s
 
 // Only function templates
 [[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Frontend/sycl-aux-triple.cpp
===
--- clang/test/Frontend/sycl-aux-triple.cpp
+++ clang/test/Frontend/sycl-aux-triple.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
 
 // CHECK-NOT:#define __x86_64__ 1
 // CHECK-SYCL:#define __x86_64__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::Undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > This option also must be controlled by `-fsycl`:
> > ```
> > Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
> > 
> > ```
> Does it really has to? This logic is already present in the driver and it 
> makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> Can `-fsycl-is-device` imply `-fsycl`?
> Looking how CUDA/OpenMP options are handled, not all of them are processed 
> using this pattern.
In general, this is how we handle it in OpenMP. Cuda works differently, because 
it has its own kind of files (.cu) and Cuda is triggered by the language switch 
(-x cu). Seems to me, you're using something close to OpenMP model, no? Or do 
you want to define your own language kind just like Cuda?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> This option also must be controlled by `-fsycl`:
> ```
> Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
> 
> ```
Does it really has to? This logic is already present in the driver and it makes 
front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
Can `-fsycl-is-device` imply `-fsycl`?
Looking how CUDA/OpenMP options are handled, not all of them are processed 
using this pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2547
 
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);

`-fno-sycl` should not be passed to frontend. Just use `hasFlag(OPT_fsycl)`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

This option also must be controlled by `-fsycl`:
```
Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

bader wrote:
> ABataev wrote:
> > These flags should not be ignored, `NoArgumentUnused` should be applied to 
> > this flags.
> Do you mean "`NoArgumentUnused` should **not** be applied"?
Yes, missed `not`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

ABataev wrote:
> These flags should not be ignored, `NoArgumentUnused` should be applied to 
> this flags.
Do you mean "`NoArgumentUnused` should **not** be applied"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 243173.
bader marked 2 inline comments as done.
bader added a comment.

Applied code review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::Undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,26 @@
   LangStd = OpenCLLangStd;
   }
 
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {
+// -sycl-std applies to any SYCL source, not only those containing kernels,
+// but also those using the SYCL API
+if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+  Opts.setSYCLVersion(
+  llvm::StringSwitch(A->getValue())
+  .Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+ LangOptions::SYCLVersionList::SYCL_2015)
+  .Default(LangOptions::SYCLVersionList::Undefined));
+
+  if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::Undefined) {
+// User has passed an invalid value to the flag, this is an error
+Diags.Report(diag::err_drv_invalid_value)
+<< A->getAsString(Args) << A->getValue();
+  }
+}
+  }
+
   

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

These flags should not be ignored, `NoArgumentUnused` should be applied to this 
flags.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5304-5306
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+

This code is not required, you already forwarded `sycl-std` to the frontend 
earlier



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2549
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(

I think processing of `sycl-std` in the frontend also must be controlled by 
some high-level option, like `-fsycl` or something like this. Without this 
`-fsycl`-like option this `std` option also must be ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 243029.
bader added a comment.

Ignore `-sycl-std` if it used in non-SYCL mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::Undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,22 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::Undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::Undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4030-4031
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > Should this option also be controlled by `-fsycl`? 
> > > Make sense to me.
> > > @Ruyk, are you okay with that?
> > > 
> > > @ABataev, do you have any suggestion on what we should we do if 
> > > `-sycl-std` option is used w/o `-fsycl`? Ignore or report warning/error?
> > Mark this option as `NoArgumentUnused` (so the compiler does not report 
> > unused option) and ignore it.
> Ok. Is it okay if we add one more flag - `CoreOption`? We'd like enable these 
> options fir `clang-cl` as well.
Hard to say, need to look at the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4030-4031
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > Should this option also be controlled by `-fsycl`? 
> > Make sense to me.
> > @Ruyk, are you okay with that?
> > 
> > @ABataev, do you have any suggestion on what we should we do if `-sycl-std` 
> > option is used w/o `-fsycl`? Ignore or report warning/error?
> Mark this option as `NoArgumentUnused` (so the compiler does not report 
> unused option) and ignore it.
Ok. Is it okay if we add one more flag - `CoreOption`? We'd like enable these 
options fir `clang-cl` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4030-4031
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {

bader wrote:
> ABataev wrote:
> > Should this option also be controlled by `-fsycl`? 
> Make sense to me.
> @Ruyk, are you okay with that?
> 
> @ABataev, do you have any suggestion on what we should we do if `-sycl-std` 
> option is used w/o `-fsycl`? Ignore or report warning/error?
Mark this option as `NoArgumentUnused` (so the compiler does not report unused 
option) and ignore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader marked 7 inline comments as done.
bader added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4030-4031
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {

ABataev wrote:
> Should this option also be controlled by `-fsycl`? 
Make sense to me.
@Ruyk, are you okay with that?

@ABataev, do you have any suggestion on what we should we do if `-sycl-std` 
option is used w/o `-fsycl`? Ignore or report warning/error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4030-4031
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {

Should this option also be controlled by `-fsycl`? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 242909.
bader marked 2 inline comments as done.
bader added a comment.

Applied Alexey's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,5 +1,9 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
@@ -7,4 +11,6 @@
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::Undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,22 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::Undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::Undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,9 +4023,17 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
-  if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false))
+  bool IsSYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  if (IsSYCL)
 CmdArgs.push_back("-fsycl-is-device");
 
+  if (Arg *A 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:122
 
+  enum class SYCLVersionList { SYCL_2015, SYCL_1_2_1 = SYCL_2015, undefined };
+

s/undefined/Undefined/g



Comment at: clang/include/clang/Basic/LangOptions.h:122
 
+  enum class SYCLVersionList { SYCL_2015, SYCL_1_2_1 = SYCL_2015, undefined };
+

ABataev wrote:
> s/undefined/Undefined/g
Do you use `SYCL_1_2_1` anywhere in the code? I don't see it is used and you 
can drop this enum.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2552-2553
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::undefined));

It does not match the list of values in `Options.td` file



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2562
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);

Can `OPT_fsycl` flag be ever passed to the frontend? Also, seems to me the 
driver has a special case already for `device` mode, so this code must be the 
dead code, actually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon check-circle color=green} clang-tidy: pass.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 242845.
bader added a comment.

Applied suggestions from Alexey and Ruyman and rebased on ToT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL,CHECK-SYCL-STD %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,25 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,9 +4023,17 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
-  if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false))
+  bool IsSYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  if (IsSYCL)
 CmdArgs.push_back("-fsycl-is-device");
 
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {
+// Ensure the default version in SYCL mode is 1.2.1 (aka 2015)
+CmdArgs.push_back("-sycl-std=2015");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
@@ -5293,6 +5301,9 @@
options::OPT_fno_hip_new_launch_api, false))
 CmdArgs.push_back("-fhip-new-launch-api");
 
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+
   if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
 CmdArgs.push_back(
 Args.MakeArgString(Twine("-fcf-protection=") + A->getValue()));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3415,6 +3415,8 @@
   HelpText<"Enable SYCL kernels 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Ruyman via Phabricator via cfe-commits]

Ruyk added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:206
 LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "C++ for OpenCL version")
+ENUM_LANGOPT(SYCLVersion, SYCLVersionList, 4, SYCLVersionList::undefined, 
"Version of the SYCL standard used")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")

bader wrote:
> All other language options controlling standard versions are added as 
> "LANGOPT" i.e. `int`. Why SYCLVersion is different?
> @Ruyk, do you think we should convert other options (e.g. `OpenCL`) to enums 
> as well?
Thats a good point. I don't see strong reasons for the enum, basically I read 
the comment in 
https://code.woboq.org/llvm/clang/include/clang/Basic/LangOptions.def.html#22


```
// ENUM_LANGOPT: for options that have enumeration, rather than unsigned, type.
```

And since there is a known set of SYCL specifications, made more sense to 
enumerate it.
It also simplifies writing variants to 1.2.1 (e.g. 1.2.1-oneapi) in the code 
since then you can add another entry to the enum.

But no strong feelings, so feel free to change it.




Comment at: clang/include/clang/Driver/Options.td:3401
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"1.2.1">;
 

bader wrote:
> What do you think we integrate sycl versions to existing clang options 
> controlling language version: `-std`.
> As far as I can see it's used for all the C/C+ extensions like 
> OpenMP/OpenCL/CUDA/HIP/ObjC.
> 
> If I understand correctly clang supports `-cl-std` only because it's required 
> by OpenCL standard. Similar option (i.e. `-sycl-std`) is not required by the 
> SYCL specification, so using `-std` is more aligned with existing clang 
> design.
> 
> See clang/include/clang/Basic/LangStandard.h and 
> clang/include/clang/Basic/LangStandards.def.
In the case of SYCL, you may want to compile your code with C++17 and SYCL 
2015, in which case you need both -std=c++17 and -sycl=sycl-2015 . 
SYCL specification mandates a minimum C++ version but users can write code on 
newer versions as long as the code in the kernel scope is still valid.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3989
+// Ensure the default version in SYCL mode is 1.2.1
+CmdArgs.push_back("-sycl-std=1.2.1");
+  }

This should probably change to 2015 if we are allowing that, just for 
consistency


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3412-3414
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,

bader wrote:
> ABataev wrote:
> > Better to split this into 2 parts: the first for `-fsycl` and the second 
> > for `-sycl-std=`.
> Okay. Just one question: 
> 
> Do you know if it's okay to have two commits per review request (like the 
> first version of the "patch" in this review - it was split as you suggest)? 
> Or I should create two separate review requests?
Separate patches must have separate review requests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:3412-3414
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,

ABataev wrote:
> Better to split this into 2 parts: the first for `-fsycl` and the second for 
> `-sycl-std=`.
Okay. Just one question: 

Do you know if it's okay to have two commits per review request (like the first 
version of the "patch" in this review - it was split as you suggest)? Or I 
should create two separate review requests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3412-3414
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,

Better to split this into 2 parts: the first for `-fsycl` and the second for 
`-sycl-std=`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62372 tests passed, 0 failed 
and 839 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 241872.
bader added a comment.

Fix clang-format and clang-tidy issues reported by merge_guards_bot


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL,CHECK-SYCL-STD %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2545,6 +2545,25 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4010,6 +4010,18 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {
+// Ensure the default version in SYCL mode is 1.2.1
+CmdArgs.push_back("-sycl-std=1.2.1");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
@@ -5259,6 +5271,9 @@
options::OPT_fno_hip_new_launch_api, false))
 CmdArgs.push_back("-fhip-new-launch-api");
 
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+
   if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62372 tests passed, 0 failed 
and 839 were skipped.

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 241757.
bader added a comment.

Applied suggestion from Ruyman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL,CHECK-SYCL-STD %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,18 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::undefined:
+  default:
+// This is not a SYCL source, nothing to add
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2545,6 +2545,25 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4010,6 +4010,18 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {
+// Ensure the default version in SYCL mode is 1.2.1
+CmdArgs.push_back("-sycl-std=1.2.1");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
@@ -5259,6 +5271,9 @@
options::OPT_fno_hip_new_launch_api, false))
 CmdArgs.push_back("-fhip-new-launch-api");
 
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+
   if (Arg *A = 

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader marked 2 inline comments as done.
bader added a comment.

> Maybe we should use the year of issue (2015 instead of 1.2.1) for the 
> -sycl-std version? That would be more stable for the upcoming SYCL versions, 
> and match somehow the C++ versioning.

Sounds good to me. I'll update the patch. I also have a couple of design 
related questions below.




Comment at: clang/include/clang/Basic/LangOptions.def:206
 LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "C++ for OpenCL version")
+ENUM_LANGOPT(SYCLVersion, SYCLVersionList, 4, SYCLVersionList::undefined, 
"Version of the SYCL standard used")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")

All other language options controlling standard versions are added as "LANGOPT" 
i.e. `int`. Why SYCLVersion is different?
@Ruyk, do you think we should convert other options (e.g. `OpenCL`) to enums as 
well?



Comment at: clang/include/clang/Driver/Options.td:3401
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"1.2.1">;
 

What do you think we integrate sycl versions to existing clang options 
controlling language version: `-std`.
As far as I can see it's used for all the C/C+ extensions like 
OpenMP/OpenCL/CUDA/HIP/ObjC.

If I understand correctly clang supports `-cl-std` only because it's required 
by OpenCL standard. Similar option (i.e. `-sycl-std`) is not required by the 
SYCL specification, so using `-std` is more aligned with existing clang design.

See clang/include/clang/Basic/LangStandard.h and 
clang/include/clang/Basic/LangStandards.def.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Ruyman via Phabricator via cfe-commits]

Ruyk added a comment.

Maybe we should use the year of issue (2015 instead of 1.2.1) for the -sycl-std 
version? That would be more stable for the upcoming SYCL versions, and match 
somehow the C++ versioning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
Herald added subscribers: cfe-commits, Anastasia, ebevhan.
Herald added a project: clang.

User can select the version of SYCL the compiler will
use via the flag -sycl-std, similar to -cl-std.

The flag defines the LangOpts.SYCLVersion option to the
version of SYCL. The default value is undefined.
If driver is building SYCL code, flag is set to the default SYCL
version (1.2.1)

The preprocessor uses this variable to define CL_SYCL_LANGUAGE_VERSION macro,
which should be defined according to SYCL 1.2.1 standard.

Only valid value at this point for the flag is 1.2.1.

Co-Authored-By: David Wood 
Signed-off-by: Ruyman Reyes 
Signed-off-by: Alexey Bader 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL,CHECK-SYCL-STD %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,18 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::sycl_1_2_1:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::undefined:
+  default:
+// This is not a SYCL source, nothing to add
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2513,6 +2513,25 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::sycl_1_2_1)
+.Default(LangOptions::SYCLVersionList::undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::sycl_1_2_1);
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3977,6 +3977,18 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {
+// Ensure the default version in SYCL mode is 1.2.1
+CmdArgs.push_back("-sycl-std=1.2.1");
+  }