-
Notifications
You must be signed in to change notification settings - Fork 769
[Driver][SYCL][FPGA] Enable dependency file usage from static archive… #2443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Driver][SYCL][FPGA] Enable dependency file usage from static archive… #2443
Conversation
1dda16f
to
5ed1c55
Compare
…s for FPGA When performing compilations for FPGA, we want to be sure to take advantage of dependency information that could be part of the fat static archives.
5ed1c55
to
0b9e33e
Compare
/// Check for unbundle and use of deps in static lib | ||
// RUN: %clang_cl --target=x86_64-pc-windows-msvc -fsycl -fintelfpga %t.lib -### 2>&1 \ | ||
// RUN: | FileCheck -check-prefix=CHECK_UNBUNDLE %s | ||
// CHECK_UNBUNDLE: clang-offload-bundler" "-type=aoo" "-targets=sycl-fpga_dep" "-inputs={{.*}}" "-outputs=[[DEPFILES:.+\.txt]]" "-unbundle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, why is it that separate unbundler calls are being made using the same archive instead of a single one as a comma? I can't quite trace this back in the code...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unbundling of the dep files is separate from the unbundle call after the partial-link. The behavior is similar to what we do for grabbing the dep files for objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might've formulated this incorrectly - didn't mean this thread to be strictly focused on the current patch :) More of a "why do we have to have separate unbundler calls at all if we're able to get multiple outputs from a single one". But I have a vague idea that the Action
construction could be problematic, given the nature of the hack that allows us to have multiple unbundler outputs at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. The additional dependency target doesn't fit within the regular unbundler behavior expectations (full toolchains for each target), so we add the dep file extraction as a singular input to the offline compile. There may be a way to accomplish this, but would probably require a bit more investigation/rework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other parts looks good to me :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change fixes cases when the input function name does not match OpenCL spec but starts with `convert_` prefix, e.g. `convert_float_42`. Original commit: KhronosGroup/SPIRV-LLVM-Translator@848a6d7af70ba28
Check that builtin is valid mostly by matching the regular expression - remove unneeded checks. This is a follow-up to #2443 Original commit: KhronosGroup/SPIRV-LLVM-Translator@04144823462c6c4
[benchmarks] adjust label and legend positions on bar charts
…s for FPGA
When performing compilations for FPGA, we want to be sure to take advantage
of dependency information that could be part of the fat static archives.