Repository navigation
Conversation
| static constexpr uint64_t SYCLHugeDeviceCodeWarnThreshold = 1ull << 30; // 1GB | ||
|
|
||
| // Best-effort: skips non-filename inputs (e.g. linker flags). | ||
| static uint64_t getTotalSYCLDeviceCodeSize(ArrayRef<InputInfo> Inputs) { |
There was a problem hiding this comment.
This is grabbing size information for the entire object/file. This may not be representative of the actual device code. Consider renaming the function name to better match what is being checked.
There was a problem hiding this comment.
Renamed to getTotalLinkInputSize
elizabethandrews
left a comment
elizabethandrews
left a comment
There was a problem hiding this comment.
This is primarily a Driver Change with just a warning group declared in FE. I defer to @mdtoguchi for review. I agree that the current proposed warning is too verbose.
mdtoguchi
changed the title
| static constexpr uint64_t HugeLinkInputWarnThreshold = 1ull << 30; // 1GiB | ||
|
|
||
| // Best-effort: skips non-filename inputs (e.g. linker flags). | ||
| static uint64_t getTotalLinkInputSize(ArrayRef<InputInfo> Inputs) { |
There was a problem hiding this comment.
My main concern here is this warning may be triggered too much especially when considering any inputs that are not object related and are associated with static archives. The check is basically all user input files, regardless if they impact the device binary size or not. Would it be better to check the final device binary size after the device link and emit the warning then if the device binary is too big?
There was a problem hiding this comment.
Ok Fixed. I made it such that we only measure the actually wrapped device-code object size.
… bviyer-make-better-warning
|
|
||
| for (const InputInfo &II : Inputs) | ||
| if (II.isFilename() && | ||
| isa_and_nonnull<OffloadWrapperJobAction>(II.getAction())) { |
There was a problem hiding this comment.
This is good for the old offload model. We should also look into a solution for the new model - I would expect that to be more in the clang-linker-wrapper space.
There was a problem hiding this comment.
Can we do that in a separate PR?
There was a problem hiding this comment.
Yup, this can be done in a separate PR.
| static constexpr uint64_t HugeDeviceCodeWarnThreshold = 1ull << 30; // 1GiB | ||
|
|
||
| // Sums the size of only those link inputs that carry wrapped device code, | ||
| // i.e. skips host objects, static archives, and other linker inputs that |
There was a problem hiding this comment.
the comment on static archives isn't true - as static archives should be queried as well, as they can be 'fat archives'. But that query happens in a different area so the device objects from any static archives should already be processed along with the other device binaries.
Large device code can push the 32-bit PC relative relocations past their 2GB reach. this causes host linker to fail with errors such as "relocation truncated to fit." In this PR, such cases are dignosed at linked time via
-Wlarge-device-kernelflag and providing user suggestion to use theflink-huge-device-codeas a fix.