[SYCL] Add clang front-end option to enable SYCL device compilation flow.

[bader]

Summary

Patch by Mariya Podchishchaeva <[e-mail]>

[keryell]

That looks good. Thanks.

[Naghasan]

LGTM

Side note: might be good to also involve @Anastasia, as some of the future patches will overlap with OpenCL.

[bader]

In D57768#1386754, @Naghasan wrote:

LGTM

Side note: might be good to also involve @Anastasia, as some of the future patches will overlap with OpenCL.

Sure. I'll add @Anastasia as a reviewer to the relevant patches.

Thanks,
Alexey

[Anastasia]

In D57768#1386791, @bader wrote:

In D57768#1386754, @Naghasan wrote:

LGTM

Side note: might be good to also involve @Anastasia, as some of the future patches will overlap with OpenCL.

Sure. I'll add @Anastasia as a reviewer to the relevant patches.

Thanks,
Alexey

Hi,

Sorry for the delay. I was planning to reply on the RFC this week after finishing looking at your prototype in github. Seems quite a substantial amount of work!

There are a number of big architectural aspects of Clang that this work affects. My personal feeling is that some more senior developers in Clang architecture should provide feedback before we go ahead with detailed reviews.

Particularly, the following needs clarification:

• SYCL seem to require adding tight dependencies from the standard libraries into the compiler because many language features are hidden behind library classes. This is not common for Clang. We had a discussion about this issue during the implementation of OpenCL C++ and it was decided not to go this route for upstream Clang. Can you explain your current approach to implement this? I think @rjmccall or @rsmith might need to be involved in this.

• I am not sure how the change of direction for OpenCL C++ to just enabling C++ in OpenCL would affect your work now? Particularly we are establishing a lot of rules in the areas of interplay between OpenCL features and C++. Address space handling is one example here. As far as I am aware SYCL doesn't detail many of these rules either. So I am wondering how it would work... would you just inherit the same rules? Also keep in mind they are not documented anywhere yet other than the source code.

• What is your solution for integration with SPIR-V and how does it relate to our previous discussions in October: http://lists.llvm.org/pipermail/cfe-dev/2018-October/059974.html

• Can you explain the purpose of https://github.com/intel/llvm/tree/sycl/clang/lib/CodeGen/OclCxxRewrite that you are adding to Clang?

I will follow up on RFC as well so we can have a wider discussion.

[ABataev]

This definitely requires a test.

[bader]

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

[ABataev]

In D57768#1386933, @bader wrote:

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

There are several similar tests:
OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There is no absolutely the same test for OpenMP, since OpenMP has mo similar req for the offloading.

[rjmccall]

In D57768#1386862, @Anastasia wrote:

• SYCL seem to require adding tight dependencies from the standard libraries into the compiler because many language features are hidden behind library classes. This is not common for Clang. We had a discussion about this issue during the implementation of OpenCL C++ and it was decided not to go this route for upstream Clang. Can you explain your current approach to implement this? I think @rjmccall or @rsmith might need to be involved in this.

I'd like to know more about this, but I'll point out that this isn't unprecedented:

• C compilers have hard-coded knowledge about va_list.
• C++ compilers have hard-coded knowledge about std::type_info and std::initializer_list (and possibly others I've forgotten).

Whether that's the right direction for SYCL, though, I can't say until I understand more about what dependencies are being proposed.

[bader]

In D57768#1386941, @ABataev wrote:

In D57768#1386933, @bader wrote:

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

There are several similar tests:
OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There is no absolutely the same test for OpenMP, since OpenMP has mo similar req for the offloading.

@ABataev thanks for the pointers. The uploaded patch adds two options:

• fsycl-is-device (front-end option)
• sycl-device-only (driver option)

The driver tests you mention validate driver logic enabled by new options, which is not part of this test and I was going to add it later.
I can split the patch and remove new driver option and leave only front-end option.
Another option is to add driver logic that invokes the front-end compiler in "device only" mode.
Which option do you prefer?

[ABataev]

In D57768#1392975, @bader wrote:

In D57768#1386941, @ABataev wrote:

In D57768#1386933, @bader wrote:

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

There are several similar tests:
OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There is no absolutely the same test for OpenMP, since OpenMP has mo similar req for the offloading.

@ABataev thanks for the pointers. The uploaded patch adds two options:

• fsycl-is-device (front-end option)
• sycl-device-only (driver option)

The driver tests you mention validate driver logic enabled by new options, which is not part of this test and I was going to add it later.
I can split the patch and remove new driver option and leave only front-end option.
Another option is to add driver logic that invokes the front-end compiler in "device only" mode.
Which option do you prefer?

Yes, split the patch into 2. But you still need to add the tests for each of them.
You need to add at least 2 tests:

1. The test that checks that the driver option is converted into the frontend option (driver with --sycl-device-only calls the frontend with -fsycl-is-device.
2. The test that checks that the frontend option -fsycl-is-device adds a new macro definition __SYCL_DEVICE_ONLY__ 1

[Anastasia]

In D57768#1389012, @rjmccall wrote:

In D57768#1386862, @Anastasia wrote:

• SYCL seem to require adding tight dependencies from the standard libraries into the compiler because many language features are hidden behind library classes. This is not common for Clang. We had a discussion about this issue during the implementation of OpenCL C++ and it was decided not to go this route for upstream Clang. Can you explain your current approach to implement this? I think @rjmccall or @rsmith might need to be involved in this.

I'd like to know more about this, but I'll point out that this isn't unprecedented:

• C compilers have hard-coded knowledge about va_list.
• C++ compilers have hard-coded knowledge about std::type_info and std::initializer_list (and possibly others I've forgotten).

Whether that's the right direction for SYCL, though, I can't say until I understand more about what dependencies are being proposed.

In OpenCL we hard-coded printf btw! I think it would be good to understand how much of those is needed for SYCL. The fact that the entire language is implemented as a library is a bit worrying. But hopefully we can already map most of the things to some existing language constructs (address space qualifiers, function attributes, etc...).

[bader]

Applied comments from @ABataev.

• Split changes into two patches. This part contains front-end option enabling device specific macro. Changes adding driver option will be sent in a separate patch.
• Added LIT test

[ABataev]

clang/test/Preprocessor/sycl-macro.cpp

1 ↗

(On Diff #187751)

Add a test that without this option __SYCL_DEVICE_ONLY__ is not defined

[bader]

Add check that SYCL specific macro is not enabled by default.

[ABataev]

LG

[bader]

Committed: https://reviews.llvm.org/rL354773

clang/include/clang/Basic/LangOptions.def

 LANGOPT(RenderScript      , 1, 0, "RenderScript")
 
 LANGOPT(CUDAIsDevice      , 1, 0, "compiling for CUDA device")
 LANGOPT(CUDAAllowVariadicFunctions, 1, 0, "allowing variadic functions in CUDA device code")
 LANGOPT(CUDAHostDeviceConstexpr, 1, 1, "treating unattributed constexpr functions as __host__ __device__")
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice      , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are unavailable")
 LANGOPT(NewAlignOverride  , 32, 0, "maximum alignment guaranteed by '::operator new(size_t)'")
 LANGOPT(ConceptsTS , 1, 0, "enable C++ Extensions for Concepts")
 BENIGN_LANGOPT(ModulesCodegen , 1, 0, "Modules code generation")
 BENIGN_LANGOPT(ModulesDebugInfo , 1, 0, "Modules debug info")
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")

clang/include/clang/Driver/CC1Options.td

 // OpenMP Options
 //===----------------------------------------------------------------------===//
 
 def fopenmp_is_device : Flag<["-"], "fopenmp-is-device">,
   HelpText<"Generate code only for an OpenMP target device.">;
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===----------------------------------------------------------------------===//
+// SYCL Options
+//===----------------------------------------------------------------------===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 //===----------------------------------------------------------------------===//
 // cc1as-only Options
 //===----------------------------------------------------------------------===//
 
 let Flags = [CC1AsOption, NoDriverOption] in {
 
 // Language Options

clang/include/clang/Driver/Options.td

 defm recursive : BooleanFFlag<"recursive">, Group<gfortran_Group>;
 defm repack_arrays : BooleanFFlag<"repack-arrays">, Group<gfortran_Group>;
 defm second_underscore : BooleanFFlag<"second-underscore">, Group<gfortran_Group>;
 defm sign_zero : BooleanFFlag<"sign-zero">, Group<gfortran_Group>;
 defm stack_arrays : BooleanFFlag<"stack-arrays">, Group<gfortran_Group>;
 defm underscoring : BooleanFFlag<"underscoring">, Group<gfortran_Group>;
 defm whole_file : BooleanFFlag<"whole-file">, Group<gfortran_Group>;
 
+// SYCL options
+def sycl_device_only : Flag<["--"], "sycl-device-only">,
+  HelpText<"Compile SYCL code for device only">;
 
 include "CC1Options.td"
 
 include "CLCompatOptions.td"

clang/lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
   // found
   if (Arg *A = Args.getLastArg(options::OPT_fopenmp_host_ir_file_path)) {
     Opts.OMPHostIRFile = A->getValue();
     if (!llvm::sys::fs::exists(Opts.OMPHostIRFile))
       Diags.Report(diag::err_drv_omp_host_ir_file_not_found)
           << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
                         Args.hasArg(options::OPT_fopenmp_cuda_mode);
 
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAForceFullRuntime =
       Opts.OpenMPIsDevice && T.isNVPTX() &&
       Args.hasArg(options::OPT_fopenmp_cuda_force_full_runtime);

clang/lib/Frontend/InitPreprocessor.cpp

#undef DEFINE_LOCK_FREE_MACRO
   }
 
   // We need to communicate this to our CUDA header wrapper, which in turn
   // informs the proper CUDA headers of this choice.
   if (LangOpts.CUDADeviceApproxTranscendentals || LangOpts.FastMath) {
     Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+    Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
     if (TI.getSupportedOpenCLOpts().isSupported(#Ext, \
         LangOpts.OpenCLVersion)) \
       Builder.defineMacro(#Ext);
 #include "clang/Basic/OpenCLExtensions.def"