Skip to content

Commit f177937

Browse files
doru1004ronlieb
authored andcommitted
Reland upstream patch: [Libomptarget] Rework image checking further
Change-Id: I0e58f6f4a38ab17d9657e9eeded8006eec9d8a66
1 parent bdb854e commit f177937

File tree

6 files changed

+42
-46
lines changed

6 files changed

+42
-46
lines changed

openmp/libomptarget/plugins-nextgen/common/include/JIT.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct JITEngine {
5757

5858
/// Return true if \p Image is a bitcode image that can be JITed for the given
5959
/// architecture.
60-
bool checkBitcodeImage(const __tgt_device_image &Image);
60+
Expected<bool> checkBitcodeImage(StringRef Buffer) const;
6161

6262
private:
6363
/// Compile the bitcode image \p Image and generate the binary image that can

openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ struct GenericPluginTy {
11671167

11681168
/// Top level interface to verify if a given ELF image can be executed on a
11691169
/// given target. Returns true if the \p Image is compatible with the plugin.
1170-
Expected<bool> checkELFImage(__tgt_device_image &Image) const;
1170+
Expected<bool> checkELFImage(StringRef Image) const;
11711171

11721172
/// Indicate if an image is compatible with the plugin devices. Notice that
11731173
/// this function may be called before actually initializing the devices. So

openmp/libomptarget/plugins-nextgen/common/src/JIT.cpp

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -330,24 +330,18 @@ JITEngine::process(const __tgt_device_image &Image,
330330
return &Image;
331331
}
332332

333-
bool JITEngine::checkBitcodeImage(const __tgt_device_image &Image) {
333+
Expected<bool> JITEngine::checkBitcodeImage(StringRef Buffer) const {
334334
TimeTraceScope TimeScope("Check bitcode image");
335335

336-
if (!isImageBitcode(Image))
337-
return false;
338-
339-
StringRef Data(reinterpret_cast<const char *>(Image.ImageStart),
340-
target::getPtrDiff(Image.ImageEnd, Image.ImageStart));
341-
auto MB = MemoryBuffer::getMemBuffer(Data, /*BufferName=*/"",
342-
/*RequiresNullTerminator=*/false);
343-
if (!MB)
344-
return false;
336+
assert(identify_magic(Buffer) == file_magic::bitcode &&
337+
"Input is not bitcode");
345338

346339
LLVMContext Context;
347-
SMDiagnostic Diagnostic;
348-
std::unique_ptr<Module> M =
349-
llvm::getLazyIRModule(std::move(MB), Diagnostic, Context,
350-
/*ShouldLazyLoadMetadata=*/true);
340+
auto ModuleOrErr = getLazyBitcodeModule(MemoryBufferRef(Buffer, ""), Context,
341+
/*ShouldLazyLoadMetadata=*/true);
342+
if (!ModuleOrErr)
343+
return ModuleOrErr.takeError();
344+
Module &M = **ModuleOrErr;
351345

352-
return M && Triple(M->getTargetTriple()).getArch() == TT.getArch();
346+
return Triple(M.getTargetTriple()).getArch() == TT.getArch();
353347
}

openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,24 +1731,21 @@ Error GenericPluginTy::deinitDevice(int32_t DeviceId) {
17311731
return Plugin::success();
17321732
}
17331733

1734-
Expected<bool> GenericPluginTy::checkELFImage(__tgt_device_image &Image) const {
1735-
StringRef Buffer(reinterpret_cast<const char *>(Image.ImageStart),
1736-
target::getPtrDiff(Image.ImageEnd, Image.ImageStart));
1737-
1734+
Expected<bool> GenericPluginTy::checkELFImage(StringRef Image) const {
17381735
// First check if this image is a regular ELF file.
1739-
if (!utils::elf::isELF(Buffer))
1736+
if (!utils::elf::isELF(Image))
17401737
return false;
17411738

17421739
// Check if this image is an ELF with a matching machine value.
1743-
auto MachineOrErr = utils::elf::checkMachine(Buffer, getMagicElfBits());
1740+
auto MachineOrErr = utils::elf::checkMachine(Image, getMagicElfBits());
17441741
if (!MachineOrErr)
17451742
return MachineOrErr.takeError();
17461743

17471744
if (!*MachineOrErr)
17481745
return false;
17491746

17501747
// Perform plugin-dependent checks for the specific architecture if needed.
1751-
return isELFCompatible(Buffer);
1748+
return isELFCompatible(Image);
17521749
}
17531750

17541751
const bool llvm::omp::target::plugin::libomptargetSupportsRPC() {
@@ -1778,27 +1775,38 @@ int32_t __tgt_rtl_init_plugin() {
17781775
return OFFLOAD_SUCCESS;
17791776
}
17801777

1781-
int32_t __tgt_rtl_is_valid_binary(__tgt_device_image *TgtImage) {
1782-
// TODO: We should be able to perform a trivial ELF machine check without
1783-
// initializing the plugin first to save time if the plugin is not needed
1778+
int32_t __tgt_rtl_is_valid_binary(__tgt_device_image *Image) {
17841779
if (!Plugin::isActive())
17851780
return false;
17861781

1787-
// Check if this is a valid ELF with a matching machine and processor.
1788-
auto MatchOrErr = Plugin::get().checkELFImage(*TgtImage);
1789-
if (Error Err = MatchOrErr.takeError()) {
1782+
StringRef Buffer(reinterpret_cast<const char *>(Image->ImageStart),
1783+
target::getPtrDiff(Image->ImageEnd, Image->ImageStart));
1784+
1785+
auto HandleError = [&](Error Err) -> bool {
17901786
[[maybe_unused]] std::string ErrStr = toString(std::move(Err));
1791-
DP("Failure to check validity of image %p: %s", TgtImage, ErrStr.c_str());
1787+
DP("Failure to check validity of image %p: %s", Image, ErrStr.c_str());
1788+
return false;
1789+
};
1790+
switch (identify_magic(Buffer)) {
1791+
case file_magic::elf:
1792+
case file_magic::elf_relocatable:
1793+
case file_magic::elf_executable:
1794+
case file_magic::elf_shared_object:
1795+
case file_magic::elf_core: {
1796+
auto MatchOrErr = Plugin::get().checkELFImage(Buffer);
1797+
if (Error Err = MatchOrErr.takeError())
1798+
return HandleError(std::move(Err));
1799+
return *MatchOrErr;
1800+
}
1801+
case file_magic::bitcode: {
1802+
auto MatchOrErr = Plugin::get().getJIT().checkBitcodeImage(Buffer);
1803+
if (Error Err = MatchOrErr.takeError())
1804+
return HandleError(std::move(Err));
1805+
return *MatchOrErr;
1806+
}
1807+
default:
17921808
return false;
1793-
} else if (*MatchOrErr) {
1794-
return true;
17951809
}
1796-
1797-
// Check if this is a valid LLVM-IR file with matching triple.
1798-
if (Plugin::get().getJIT().checkBitcodeImage(*TgtImage))
1799-
return true;
1800-
1801-
return false;
18021810
}
18031811

18041812
void __tgt_rtl_check_invalid_image(__tgt_device_image *InvalidImage) {

openmp/libomptarget/plugins-nextgen/common/src/Utils/ELF.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ bool utils::elf::isELF(StringRef Buffer) {
4141
}
4242

4343
Expected<bool> utils::elf::checkMachine(StringRef Object, uint16_t EMachine) {
44-
if (!isELF(Object))
45-
return createError("Input is not an ELF.");
44+
assert(isELF(Object) && "Input is not an ELF!");
4645

4746
Expected<ELF64LEObjectFile> ElfOrErr =
4847
ELF64LEObjectFile::create(MemoryBufferRef(Object, /*Identifier=*/""),

revert_patches.txt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
Reverts: breaks ompt tests
2-
3-
64f0681e97c6 [Libomptarget] Rework image checking further (#76120)
4-
5-
61
Reverts: breaks HIP tests in wPSDB and nPSDB
72
[AMDGPU] Add dynamic LDS size implicit kernel argument t
83

0 commit comments

Comments
 (0)