Skip to content

[DirectX] Add only-section to llvm-objcopy for DXContainer #156372

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 8, 2025
Merged

[DirectX] Add only-section to llvm-objcopy for DXContainer #156372

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8b2734
[DirectX] Add `only-section` to `llvm-objcopy` for `DXContainer`
inbelic Sep 1, 2025
5ade083
review: clean up test case
inbelic Sep 5, 2025
8e5d4c4
review: modify test-case to check behaviour with remove
inbelic Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions llvm/lib/ObjCopy/ConfigManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,13 @@ ConfigManager::getDXContainerConfig() const {
!Common.AllocSectionsPrefix.empty() ||
Common.DiscardMode != DiscardType::None || !Common.AddSection.empty() ||
!Common.DumpSection.empty() || !Common.KeepSection.empty() ||
!Common.OnlySection.empty() || !Common.SectionsToRename.empty() ||
!Common.SetSectionAlignment.empty() || !Common.SetSectionFlags.empty() ||
!Common.SetSectionType.empty() || Common.ExtractDWO ||
Common.OnlyKeepDebug || Common.StripAllGNU || Common.StripDWO ||
Common.StripDebug || Common.StripNonAlloc || Common.StripSections ||
Common.StripUnneeded || Common.DecompressDebugSections ||
Common.GapFill != 0 || Common.PadTo != 0 ||
Common.ChangeSectionLMAValAll != 0 ||
!Common.SectionsToRename.empty() || !Common.SetSectionAlignment.empty() ||
!Common.SetSectionFlags.empty() || !Common.SetSectionType.empty() ||
Common.ExtractDWO || Common.OnlyKeepDebug || Common.StripAllGNU ||
Common.StripDWO || Common.StripDebug || Common.StripNonAlloc ||
Common.StripSections || Common.StripUnneeded ||
Common.DecompressDebugSections || Common.GapFill != 0 ||
Common.PadTo != 0 || Common.ChangeSectionLMAValAll != 0 ||
!Common.ChangeSectionAddress.empty()) {
return createStringError(
llvm::errc::invalid_argument,
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ static Error handleArgs(const CommonConfig &Config, Object &Obj) {
return Config.ToRemove.matches(P.Name);
};

if (!Config.OnlySection.empty())
RemovePred = [&Config](const Part &P) {
// Explicitly keep these sections regardless of previous removes and
// remove everything else.
Comment on lines +33 to +34
Copy link
Contributor

@bogner bogner Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should we error if both --remove-section and --only-section are specified together?

Copy link
Contributor Author

@inbelic inbelic Sep 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the same behaviour as other objcopy implementations (for instance ELFObjcopy) where --only-section should just overwrite any previous specification for it to be removed (without warning or error). I think in general we want to match the behaviour as close as we can.

Copy link
Collaborator

@jh7370 jh7370 Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a fair bit of discussion back in Phabricator at some point when bringing up llvm-objcopy for ELF. In general, the conclusion was that explicit removals override implicit keeps, while explicit keeps override implicit removals (e.g. --keep-section=X would override the implicit removal of X from --only-section=Y).

Anyway, match what ELF does (which this does)!

return !Config.OnlySection.matches(P.Name);
};

if (auto E = Obj.removeParts(RemovePred))
return E;

Expand Down
56 changes: 56 additions & 0 deletions llvm/test/tools/llvm-objcopy/DXContainer/only-section-headers.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
## Tests that the copied DXContainer correctly retains only the specified
## headers.

# RUN: yaml2obj %s -o %t
# RUN: llvm-objcopy --only-section=FKE1 --only-section=FKE4 %t %t.out
# RUN: obj2yaml %t.out | FileCheck %s

--- !dxcontainer
Header:
Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
Version:
Major: 1
Minor: 0
## FileSize = 32 (header)
## + 8 (FKE1 content) + 1688 (FKE4 content)
## + 8 (2 part offsets) + 16 (2 part headers)
## = 1752
# CHECK: FileSize: 1752
FileSize: 1996
# CHECK-NEXT: PartCount: 2
PartCount: 7
# CHECK-NEXT: PartOffsets: [ 40, 56 ]
PartOffsets: [ 60, 76, 92, 108, 236, 1932, 1960 ]
Parts:
# CHECK-NEXT: Parts
# CHECK: FKE1
# CHECK-NOT: FKE2
# CHECK-NOT: FKE3
# CHECK: FKE4
# CHECK-NOT: FKE5
# CHECK-NOT: DXIL
- Name: FKE0
Size: 8
- Name: FKE1
Size: 8
- Name: FKE2
Size: 8
- Name: FKE3
Size: 120
- Name: FKE4
Size: 1688
- Name: FKE5
Size: 20
- Name: DXIL
Size: 28
Program:
MajorVersion: 6
MinorVersion: 5
ShaderKind: 5
Size: 8
DXILMajorVersion: 1
DXILMinorVersion: 5
DXILSize: 4
DXIL: [ 0x42, 0x43, 0xC0, 0xDE, ]
...
Loading