[Driver][SYCL] Improve FPGA archive device unbundling with AOCO #9572
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ | |
| #include <set> | ||
| #include <string> | ||
| #include <system_error> | ||
| #include <unordered_set> | ||
| #include <utility> | ||
|
|
||
| using namespace llvm; | ||
|
|
@@ -71,6 +72,8 @@ using namespace clang; | |
| /// Section name which holds target symbol names. | ||
| #define SYMBOLS_SECTION_NAME ".tgtsym" | ||
|
|
||
| #define DEBUG_TYPE "clang-offload-bundler" | ||
|
|
||
| OffloadTargetInfo::OffloadTargetInfo(const StringRef Target, | ||
| const OffloadBundlerConfig &BC) | ||
| : BundlerConfig(BC) { | ||
|
|
@@ -1015,6 +1018,10 @@ class ArchiveFileHandler final : public FileHandler { | |
| .Case("a", OutputType::Archive) | ||
| .Default(OutputType::Unknown); | ||
|
|
||
| // Set contains indexes of Children that should be skipped during | ||
| // unbundling. | ||
| std::unordered_set<size_t> ExcludedChildIndexes; | ||
|
|
||
| public: | ||
| ArchiveFileHandler(const OffloadBundlerConfig &BC) : BundlerConfig(BC) {} | ||
| ~ArchiveFileHandler() = default; | ||
|
|
@@ -1029,8 +1036,10 @@ class ArchiveFileHandler final : public FileHandler { | |
| Ar = std::move(*ArOrErr); | ||
|
|
||
| // Read all children. | ||
| ssize_t ChildIndex = -1; | ||
| Error Err = Error::success(); | ||
| for (auto &C : Ar->children(Err)) { | ||
| ++ChildIndex; | ||
| auto BinOrErr = C.getAsBinary(); | ||
| if (!BinOrErr) { | ||
| if (auto Err = isNotObjectErrorInvalidFileType(BinOrErr.takeError())) | ||
|
|
@@ -1042,6 +1051,16 @@ class ArchiveFileHandler final : public FileHandler { | |
| if (!Bin->isObject()) | ||
| continue; | ||
|
|
||
| auto CheckOrErr = CheckIfObjectFileContainsExcludedTargets(C); | ||
| if (!CheckOrErr) | ||
| return CheckOrErr.takeError(); | ||
|
|
||
| if (*CheckOrErr) { | ||
| LLVM_DEBUG(outs() << "Add child to ban list. Index: " << ChildIndex | ||
| << "\n"); | ||
| ExcludedChildIndexes.emplace(ChildIndex); | ||
| } | ||
|
|
||
| auto Obj = std::unique_ptr<ObjectFile>(cast<ObjectFile>(Bin.release())); | ||
| auto Buf = MemoryBuffer::getMemBuffer(Obj->getMemoryBufferRef(), false); | ||
|
|
||
|
|
@@ -1099,7 +1118,14 @@ class ArchiveFileHandler final : public FileHandler { | |
|
|
||
| // Read all children. | ||
| Error Err = Error::success(); | ||
| ssize_t ChildIndex = -1; | ||
| for (auto &C : Ar->children(Err)) { | ||
| ++ChildIndex; | ||
| if (ExcludedChildIndexes.count(ChildIndex)) { | ||
| LLVM_DEBUG(outs() << "Skip Child. Index: " << ChildIndex << "\n"); | ||
| continue; | ||
| } | ||
|
|
||
| auto BinOrErr = C.getAsBinary(); | ||
| if (!BinOrErr) { | ||
| if (auto Err = isNotObjectErrorInvalidFileType(BinOrErr.takeError())) | ||
|
|
@@ -1215,6 +1241,70 @@ class ArchiveFileHandler final : public FileHandler { | |
| Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) override { | ||
| llvm_unreachable("unsupported for the ArchiveFileHandler"); | ||
| } | ||
|
|
||
| private: | ||
|
There was a problem hiding this comment. We are opening and parsing the file here an additional time for every child. This can prove expensive. There must be a way to do this 'exclusion' on the fly using a single parse of the file. But we can do that as a subsequent work. Please add a TODO here. There was a problem hiding this comment. I thought about this optimizations. There was a problem hiding this comment. Here it is. It removes useless work in general case. 3707aa4#diff-c29010597bfa221cf7e7cfc018e0b0a7bdae17bcf8d17b27be96f4cbd20ec28aR1293-R1295 There was a problem hiding this comment. This is surely not a blocker for this PR. But we need to address this and atleast verify that this does not increase compile time by a lot. Thanks |
||
| // NOTE: mostly a copy-paste of ReadHeader method. | ||
| Expected<std::vector<std::string>> | ||
| ReadTargetsFromChild(const Archive::Child &C) { | ||
| Expected<std::unique_ptr<Binary>> BinOrErr = C.getAsBinary(); | ||
| if (!BinOrErr) | ||
| return BinOrErr.takeError(); | ||
|
|
||
| std::unique_ptr<Binary> &Bin = BinOrErr.get(); | ||
| auto Obj = std::unique_ptr<ObjectFile>(cast<ObjectFile>(Bin.release())); | ||
| std::unique_ptr<MemoryBuffer> Buf = | ||
| MemoryBuffer::getMemBuffer(Obj->getMemoryBufferRef(), false); | ||
| ObjectFileHandler OFH(std::move(Obj), BundlerConfig); | ||
| if (Error Err = OFH.ReadHeader(*Buf)) | ||
| return Err; | ||
| Expected<std::optional<StringRef>> NameOrErr = OFH.ReadBundleStart(*Buf); | ||
| if (!NameOrErr) | ||
| return NameOrErr.takeError(); | ||
|
|
||
| std::vector<std::string> Targets; | ||
| while (*NameOrErr) { | ||
| if (*NameOrErr) | ||
| Targets.emplace_back((**NameOrErr).str()); | ||
| NameOrErr = OFH.ReadBundleStart(*Buf); | ||
| if (!NameOrErr) | ||
| return NameOrErr.takeError(); | ||
| } | ||
|
|
||
| return Targets; | ||
| } | ||
|
|
||
| bool CheckIfTargetIsExcluded(StringRef Triple) { | ||
| // NOTE: "-sycldevice" Triple component has been deprecated. | ||
| // However, it still can be met in libraries that have been compiled before | ||
| // deprecation. For example, here Triple might be the following: | ||
| // sycl-fpga_aoco-intel-unknown-sycldevice | ||
| // | ||
| // The workaround is to strip this Triple component if it is present. | ||
| Triple.consume_back("-sycldevice"); | ||
|
Comment on lines
+1277
to
+1283
There was a problem hiding this comment. We have dropped support for such binaries, no need to have that workaround anymore, see #9408 There was a problem hiding this comment. Well, it looks like FPGA libraries still depend on it. We asked them how did it happen. But it doesn't look like we can drop support of "sycldevice" env. There was a problem hiding this comment. We have effectively done that already. We do aim to preserve backwards compatibility generally, but right now we are in ABI-breaking window and we are allowed to cleanup legacy code, thus breaking backwards compatibility. |
||
| const auto &ExcludedTargetNames = BundlerConfig.ExcludedTargetNames; | ||
| auto It = std::find(ExcludedTargetNames.begin(), ExcludedTargetNames.end(), | ||
| Triple); | ||
| return It != ExcludedTargetNames.end(); | ||
| } | ||
|
|
||
| // Function reads targets from Child and checks whether one of Targets | ||
| // is in Excluded list. | ||
| Expected<bool> | ||
| CheckIfObjectFileContainsExcludedTargets(const Archive::Child &C) { | ||
| if (BundlerConfig.ExcludedTargetNames.empty()) | ||
| return false; | ||
|
|
||
| auto TargetNamesOrErr = ReadTargetsFromChild(C); | ||
| if (!TargetNamesOrErr) | ||
| return TargetNamesOrErr.takeError(); | ||
|
|
||
| auto TargetNames = TargetNamesOrErr.get(); | ||
| for (const auto &TargetName : TargetNames) | ||
| if (CheckIfTargetIsExcluded(TargetName)) | ||
| return true; | ||
|
|
||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| /// Return an appropriate object file handler. We use the specific object | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // This test prepares Archive input for clang-offload-bundler | ||
| // and checks -exclude-target command line option. | ||
| // Option should exclude fat_device_aoco object file. | ||
|
|
||
| // UNSUPPORTED: system-windows | ||
|
|
||
| // The test uses assembled archive file fatlib.a. | ||
| // The assembly algorithm is the following: | ||
| // echo "DUMMY IR FILE" > device | ||
| // echo "DUMMY IR2 FILE" > device2 | ||
| // echo "DUMMY AOCO FILE" > aoco | ||
| // echo "DUMMY HOST FILE" > host | ||
| // echo "DUMMY HOST2 FILE" > host2 | ||
| // # Wrap and compile objects | ||
| // clang-offload-wrapper -o=device.bc -host=x86_64-unknown-linux-gnu -target=spir64 -kind=sycl device | ||
| // clang-offload-wrapper -o=device2.bc -host=x86_64-unknown-linux-gnu -target=spir64 -kind=sycl device2 | ||
| // clang-offload-wrapper -o=aoco.bc -host=x86_64-unknown-linux-gnu -target=spir64 -kind=sycl aoco | ||
| // clang-offload-wrapper -o=host.bc -host=x86_64-unknown-linux-gnu -target=spir64 -kind=sycl host | ||
| // clang-offload-wrapper -o=host2.bc -host=x86_64-unknown-linux-gnu -target=spir64 -kind=sycl host2 | ||
| // llc -filetype=obj -o device.o device.bc | ||
| // llc -filetype=obj -o device2.o device2.bc | ||
| // llc -filetype=obj -o aoco.o aoco.bc | ||
| // llc -filetype=obj -o host.o host.bc | ||
| // llc -filetype=obj -o host2.o host2.bc | ||
| // # Bundle the objects | ||
| // clang-offload-bundler -input=device.o -input=host.o -output=fat_device.o -targets=sycl-spir64_fpga-unknown-unknown,host-x86_64-unknown-linux-gnu -type=o | ||
| // clang-offload-bundler -input=device2.o -input=aoco.o -input=host2.o -output=fat_device_aoco.o -targets=sycl-spir64_fpga-unknown-unknown,sycl-fpga_aoco-intel-unknown,host-x86_64-unknown-linux-gnu -type=o | ||
| // # Create the archive | ||
| // ar cr fatlib.a fat_device.o fat_device_aoco.o | ||
|
|
||
|
|
||
| // Unbundle archive | ||
| // RUN: clang-offload-bundler -type=aoo -excluded-targets=sycl-fpga_aoco-intel-unknown -targets=sycl-spir64_fpga-unknown-unknown -input=%S/Inputs/clang-offload-bundler-exclude/fatlib.a -output=%t-my_output.txt -unbundle -allow-missing-bundles | ||
|
|
||
| // Check that output of unbundling doesn't contain content of device2 | ||
| // RUN: cat %t-my_output.txt | xargs cat | strings | not grep "DUMMY IR2" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.