[SYCL] Added changes to support multiple archs in command line

[bviyer]

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa). The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

[bader]

ocloc doesn't work like that.

ocloc -device X -A -device Y -B - you assume ocloc will apply -A option to the compilation for device X and -B to the compilation for device Y. In fact, -A -B is applied to both compilations.

[sarnex]

There is the -device_options option that may do what you want:

  -device_options <device_type> <options>   Optional OpenCL C compilation options
                                            as defined by OpenCL specification - specific to a single target device.
                                            Multiple product acronyms may be provided - separated by commas.
                                            <device_type> can be product acronym or version passed in -device i.e. dg1 or 12.10.0

I didn't review the PR yet so maybe it doesn't, I just saw Alexey's comment.

[bviyer]

I think the comment might be misworded (I tried a couple things and forgot to change the comment). I also fixed another issue. Now, I think its doing the right thing:

Here is the output from clang-lnker-wrapper to ocloc:

$ ./bin/clang-linker-wrapper   --host-triple=x86_64-unknown-linux-gnu   "--device-compiler=sycl:spir64_gen-unknown-unknown=-device pvc -options -cl-mad-enable"   "--device-compiler=sycl:spir64_gen-unknown-unknown=-device skl -options -cl-unsafe-math-optimizations"  /tmp/tst_pvc.o /tmp/tst_skl.o --dry-run 2>&1 | grep ocloc
 "<snip>/ocloc" -output_no_suffix -spirv_input -device pvc -device_options pvc -ze-intel-enable-auto-large-GRF-mode -options -cl-mad-enable -output /tmp/a.out-106216.out -file /tmp/a.out-823be2.spv
 "<snip>/ocloc" -output_no_suffix -spirv_input -device skl -options -cl-unsafe-math-optimizations -output /tmp/a.out-0fad9e.out -file /tmp/a.out-d9b0a3.spv

[mdtoguchi]

The dd9abc1 change purposefully tokenizes the options to be passed to the clang-linker-wrapper. We seem to have effectively lost this behavior. Is there a reason why?

[bviyer]

Fixed.

[bader]

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa).

I think we need to fix this design issue to enable support for "multiple archs" i.e. we must use a dedicated key for each -fsycl-target= value.

The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

Joining all options into a single value to reparse them again in clang-link-wrapper tool requires implementing non-trivial logic which is a source of bugs. As Mike noted in his comment, dd9abc1 replaces this approach with simplified logic to fix one of such bugs. I suggest we don't bring it back.

[bviyer]

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa).

I think we need to fix this design issue to enable support for "multiple archs" i.e. we must use a dedicated key for each -fsycl-target= value.

The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

Joining all options into a single value to reparse them again in clang-link-wrapper tool requires implementing non-trivial logic which is a source of bugs. As Mike noted in his comment, dd9abc1 replaces this approach with simplified logic to fix one of such bugs. I suggest we don't bring it back.

I have put it back. I also added a e2e test.

[mdtoguchi]

Can a test be added that verifies the mixing of intel_gpu* and spir64_gen with -device usage where the target is the same? e.g.: -fsycl --offload-new-driver -fsycl-targets=spir64_gen,intel_gpu_skl -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" -Xsycl-target-backend=intel_gpu_skl "-options -extraopt_skl2"?

[bviyer]

added

[mdtoguchi]

Curiosity here - how was it determined to use <triple>/<arch> as opposed to some other character delimiter? I immediately jump to / being a directory separator, or even a MSVC compatible option (especially when seeing /arch usage).

[bviyer]

I just used it because it was something I used in the past and using things like "-" or "_" would collide with existing tokens - didn't give much thought other than this, tbh. Is there anything specific that you like me to use?

[mdtoguchi]

as this is more of an internal representation rather than a user facing option usage, the character used isn't of too importance as it gets the job done. I was just wondering :)