[SYCL][Doc] Update --device-compiler option and remove FPGA support from OffloadDesign.md#21037
[SYCL][Doc] Update --device-compiler option and remove FPGA support from OffloadDesign.md#21037YixingZhang007 wants to merge 8 commits intointel:syclfrom
Conversation
| resemble `--gpu-tool-arg=<arch> <arg>`. This corresponds to the existing | ||
| resemble `--device-compiler=sycl:spir64_gen-unknown-unknown==<arch> <arg>`. This corresponds to the existing | ||
| option syntax of `-fsycl-targets=intel_gpu_arch` where `arch` can be a fixed | ||
| set of targets. |
There was a problem hiding this comment.
I am not sure if this is still what we want to support, because currently, the backend compiler arguments for all architectures are passed together through a single --device-compiler= argument. For the example shown earlier in this file, if we have the following:
clang++ -fsycl -fsycl-targets=intel_gpu_skl,spir64_gen \
-Xsycl-target-backend=spir64_gen "-device pvc -options -extraopt_pvc" \
-Xsycl-target-backend=intel_gpu_skl "-options -extraopt_skl"
the clang-linker-wrapper command right now looks like:
clang-linker-wrapper ... \
--device-compiler=sycl:spir64_gen-unknown-unknown \
=-device pvc -options -extraopt_pvc -options -extraopt_skl ...
Then in clang-linker-wrapper, it will execute ocloc with both -device pvc -options -extraopt_pvc and -options -extraopt_skl for both PVC and SKL.
If we still want to keep the original proposed solution of separating the arguments for different architectures in clang-linker-wrapper, this will be something we need to implement next.
There was a problem hiding this comment.
Interesting, should not we call ocloc specifying both pvc and skl as -device options?
What does old offloading model do for this scenario?
@mdtoguchi , I believe, original design came from you, could you please comment?
There was a problem hiding this comment.
I think we need to retain this capability to allow for passing along specific values for each potential arch target. Each individual target arch provided performs a separate ocloc call.
There was a problem hiding this comment.
But does it make sense to you that we are calling ocloc with such options? -device pvc -options -extraopt_pvc -options -extraopt_skl
should not it be something like: -device pvc -options -extraopt_pvc -device skl -options -extraopt_skl?
or maybe 2 calls to ocloc?
There was a problem hiding this comment.
in other words, it looks like we are calling ocloc to compile for pvc target, while inital clang++ command line asks to compile for 2 targets: pvc and skl.
There was a problem hiding this comment.
I tried modifying the clang-linker-wrapper with two separate --device-compiler options, one for each architecture, as shown below (right now the arguments for both arch are passed through a single --device-compiler option) :
clang-linker-wrapper ... \
"--device-compiler=sycl:spir64_gen-unknown-unknown=-device pvc -options -extraopt_pvc" \
"--device-compiler=sycl:spir64_gen-unknown-unknown=-device skl -options -extraopt_skl"
The ocloc commands got called is shown below.
ocloc ... -device skl -device_options pvc -device pvc -options -extraopt_pvc -device skl -options -extraopt_skl ...
ocloc ... -device pvc -device_options pvc -device pvc -options -extraopt_pvc -device skl -options -extraopt_skl ...
I think we may still need to implement filtering logic in clang-linker-wrapper so that each --device-compiler option is only applied to its corresponding architecture @YuriPlyakhin
There was a problem hiding this comment.
yes, as we discussed on the meeting, we also need to do more experiments to better understand implemented behavior for old offloading model as well.
There was a problem hiding this comment.
I have looked into the behavior of the old offloading model for multiple devices. The argument passing into the ocloc command is different for old and new offloading models.
For example, we run the following clang command with the old offloading model:
clang++ ... -fsycl-targets=intel_gpu_dg1,spir64_gen -Xsycl-target-backend=spir64_gen "-device pvc -options -extraopt_pvc" -Xsycl-target-backend=intel_gpu_dg1 "-options -extraopt_dg1" ...
The ocloc commands run for the old offloading model are:
ocloc ... -device dg1 -device_options pvc ... -options -extraopt_dg1 ...
ocloc ... -device_options pvc -device pvc ... -options -extraopt_pvc -options -extraopt_dg1 ...
@YuriPlyakhin @mdtoguchi I don't think the ocloc commands are correct for the old offloading model, because the backend option that was passed for dg1 is also passed to pvc as well (however, the options passed to ocloc for dg1 is correct).
There was a problem hiding this comment.
hmm, how is -device_options pvc correct for dg1?
If the old offloading model is broken, I guess we can just make new offloading model to work correctly then. And we should not break any old-offloading model scenarios. So, could we implement something like what I proposed in #21037 (comment)?
and yes, for that solution additional filtering will be needed in clang-linker-wrapper based on -device ... value
There was a problem hiding this comment.
Looking at the behaviors with when mixing -fsycl-targets=spir64_gen and -fsycl-targets=intel_gpu_dg1 in your example, the driver doesn't seem to differentiate things that when assigned to spir64_gen should only go to spir64_gen explicit targets and is applying to all spir64_gen targets. Underlying triple target with intel_gpu_dg1 is spir64_gen so the driver looks to be generalizing the options at that point and passing the -Xsycl-target-backend=spir64_gen to all of the related ocloc calls.
Due to the fact that spir64_gen is more of a 'generic' value it's not clear to me if what we are doing is correct or if we should be more explicit in option passing management.
sycl/doc/design/OffloadDesign.md
Outdated
| the `spir64_gen` architecture triple, the resulting extracted binary is linked, | ||
| post-link processed and converted to SPIR-V before being passed to `ocloc` to | ||
| generate the final device binary. Options passed via `--gpu-tool-arg=` will | ||
| generate the final device binary. Options passed via `--device-compiler=` will |
There was a problem hiding this comment.
The --device-compiler usage here should be extended to include the spir64_gen target as it is specific for options to ocloc
There was a problem hiding this comment.
Thanks for the suggestion! I have update this to be --device-compiler=sycl:spir64_gen-unknown-unknown=<arg>
| > --gpu-tool-arg="-device pvc -options extraopt_pvc" | ||
| --gpu-tool-arg="-options -extraopt_skl" | ||
| > "--device-compiler=sycl:spir64_gen-unknown-unknown=-device pvc -options extraopt_pvc" | ||
| "--device-compiler=sycl:spir64_gen-unknown-unknown=-options -extraopt_skl" |
There was a problem hiding this comment.
It looks like the syntax of the options passed is slightly different (quotes around the entire option as opposed to just the arg). Was the original usage of --gpu-tool-arg not correct here?
There was a problem hiding this comment.
Yes, I think the documentation of --gpu-tool-arg here is different from what it generates when I do clang++ ... -v. Without the recent changes for --device-compiler, I see the clang-linker-wrapper command that got generated when we do clang++ ... -v is clang-linker-wrapper ... "--gpu-tool-arg=-device pvc -options -extraopt_pvc -options -extraopt_skl" ... which the quotation mark is wrapped around the whole --gpu-tool-arg option.
There was a problem hiding this comment.
Thanks - as long as the clang-linker-wrapper is parsing the information correctly how it is represented here is inconsequential.
| resemble `--gpu-tool-arg=<arch> <arg>`. This corresponds to the existing | ||
| resemble `--device-compiler=sycl:spir64_gen-unknown-unknown==<arch> <arg>`. This corresponds to the existing | ||
| option syntax of `-fsycl-targets=intel_gpu_arch` where `arch` can be a fixed | ||
| set of targets. |
There was a problem hiding this comment.
I think we need to retain this capability to allow for passing along specific values for each potential arch target. Each individual target arch provided performs a separate ocloc call.
eea246b to
008d3a2
Compare
|
Currently, support for This PR will focus solely on two changes: replacing |
This PR modifies the backend compiler options passed to clang-linker-wrapper and removes FPGA descriptions from OffloadDesign.md. Detailed explanations are below:
--device-compilerinstead of through--cpu-tool-argand--gpu-tool-arg. We update OffloadDesign.md to include the usage and format of--device-compiler.