This is an archive of the discontinued LLVM Phabricator instance.

[Binary] Support extracting offloading files from COFF
ClosedPublic

Authored by jhuber6 on Oct 27 2022, 9:17 AM.

Details

Summary

This patch adds initial support for extracting offloading binaries from
COFF objects. This is a first step to allow building offloading files
on Windows targets with the new driver.

Depends on D136796

Diff Detail

Unit TestsFailed

Event Timeline

jhuber6 created this revision.Oct 27 2022, 9:17 AM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: hiraditya. · View Herald Transcript
jhuber6 requested review of this revision.Oct 27 2022, 9:17 AM

To my eyes, this looks reasonale, but I don't know enough about COFF to be able to comfortably review this. Best get another opinion.

tra added a reviewer: rnk.Nov 1 2022, 10:31 AM
tra added a subscriber: rnk.

Added @rnk for Windows know-how.

Added @rnk for Windows know-how.

A question I have for COFF, is if the only way to identify these sections would be via the name. For ELF we use a custom section type to easily identify the fat binary section, but it doesn't seem that there is a COFF equivalent. I thought about checking for the IMAGE_SCN_LNK_REMOVE and IMAGE_SCN_MEM_DISCARDABLE flags, but that's not guaranteed to be correct.

rnk added a comment.Nov 1 2022, 12:07 PM

Seems reasonable to me.

llvm/lib/Object/OffloadBinary.cpp
72–78

Rather than duplicating the code, I think it makes sense to do the ELF check inside the loop. You can construct an ELFSectionRef from a regular SectionRef if the object is an ELF file.

jhuber6 updated this revision to Diff 472378.Nov 1 2022, 12:43 PM

Addressing comments.

rnk accepted this revision.Nov 1 2022, 3:46 PM
rnk added inline comments.
llvm/lib/Object/OffloadBinary.cpp
77

static_cast doesn't seem idiomatic, calling the constructor like ELFSectionRef(Sec).getType() != ... seems more straightforward.

This revision is now accepted and ready to land.Nov 1 2022, 3:46 PM
This revision was landed with ongoing or failed builds.Nov 3 2022, 2:19 PM
This revision was automatically updated to reflect the committed changes.