This is an archive of the discontinued LLVM Phabricator instance.

Differential D142118

[HIP] Unbundler allows missing host entry
ClosedPublic

Authored by yaxunl on Jan 19 2023, 7:45 AM.

Details

Reviewers
tra
jdoerfert
Commits
rG3006cb2aa8d9: [HIP] Unbundler allows missing host entry
Summary

clang-offload-bundler should allow missing host entry when unbundling with option
--allow-missing-bundles specified.

This is for the use cases where users want to link device-only bundles with
bundles containing both device and host entries.

Diff Detail

Unit TestsFailed

TimeTest
3,600 mslibcxx CI Modules > llvm-libc++-shared-cfg-in.libcxx/algorithms/specialized_algorithms/special_mem_concepts::nothrow_sentinel_for.compile.pass.cpp

Event Timeline

yaxunl created this revision.Jan 19 2023, 7:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 7:45 AM
yaxunl requested review of this revision.Jan 19 2023, 7:45 AM
Herald added a reviewer: jdoerfert. · View Herald TranscriptJan 19 2023, 7:45 AM
Herald added subscribers: sstefan1, MaskRay. · View Herald Transcript
Harbormaster completed remote builds in B208758: Diff 490512.Jan 19 2023, 8:54 AM
tra accepted this revision.Jan 19 2023, 2:45 PM
tra added inline comments.
clang/lib/Driver/OffloadBundler.cpp
1063

Nit: My usual pet peeve - too many negations.

if (!(FoundHostBundle || BundlerConfig.HostInputIndex == ~0u || BundlerConfig.AllowMissingBundles))) is easier to understand, IMO.

This revision is now accepted and ready to land.Jan 19 2023, 2:45 PM
yaxunl marked an inline comment as done.Jan 19 2023, 3:36 PM
yaxunl added inline comments.
clang/lib/Driver/OffloadBundler.cpp
1063

will fix when committing

This revision was landed with ongoing or failed builds.Jan 19 2023, 7:14 PM
Closed by commit rG3006cb2aa8d9: [HIP] Unbundler allows missing host entry (authored by yaxunl). · Explain Why
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
yaxunl added a commit: rG3006cb2aa8d9: [HIP] Unbundler allows missing host entry.
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 7:14 PM

Revision Contents

PathSize
clang/
lib/
Driver/
2 lines
test/
Driver/
7 lines

Diff 490512

clang/lib/Driver/OffloadBundler.cpp

Context not available.
// If we found elements, we emit an error if none of those were for the host // If we found elements, we emit an error if none of those were for the host
// in case host bundle name was provided in command line. // in case host bundle name was provided in command line.
if (!FoundHostBundle && BundlerConfig.HostInputIndex != ~0u) if (!FoundHostBundle && BundlerConfig.HostInputIndex != ~0u && !BundlerConfig.AllowMissingBundles)
traUnsubmitted
Done
ReplyInline Actions

Nit: My usual pet peeve - too many negations.

if (!(FoundHostBundle || BundlerConfig.HostInputIndex == ~0u || BundlerConfig.AllowMissingBundles))) is easier to understand, IMO.

tra: Nit: My usual pet peeve - too many negations. `if (!(FoundHostBundle || BundlerConfig.
yaxunlAuthorUnsubmitted
Done
ReplyInline Actions

will fix when committing

yaxunl: will fix when committing
return createStringError(inconvertibleErrorCode(), return createStringError(inconvertibleErrorCode(),
"Can't find bundle for the host target"); "Can't find bundle for the host target");
Context not available.

clang/test/Driver/clang-offload-bundler.c

Context not available.
// RUN: diff %t.empty %t.res.tgt2 // RUN: diff %t.empty %t.res.tgt2
// Check that bindler prints an error if given host bundle does not exist in the fat binary. // Check that bindler prints an error if given host bundle does not exist in the fat binary.
// RUN: not clang-offload-bundler -type=s -targets=host-amdgcn-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu -output=%t.res.s -output=%t.res.tgt1 -input=%t.bundle3.s -unbundle -allow-missing-bundles 2>&1 | FileCheck %s --check-prefix CK-NO-HOST-BUNDLE // RUN: not clang-offload-bundler -type=s -targets=host-amdgcn-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu -output=%t.res.s -output=%t.res.tgt1 -input=%t.bundle3.s -unbundle 2>&1 | FileCheck %s --check-prefix CK-NO-HOST-BUNDLE
// CK-NO-HOST-BUNDLE: error: Can't find bundle for the host target // CK-NO-HOST-BUNDLE: error: Can't find bundles for host-amdgcn-xxx-linux-gnu
// Check missing host entry is allowed with -allow-missing-bundles
// RUN: clang-offload-bundler -type=s -targets=host-amdgcn-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu -output=%t.res.s -output=%t.res.tgt1 -input=%t.bundle3.s -unbundle -allow-missing-bundles
// //
// Check binary bundle/unbundle. The content that we have before bundling must be the same we have after unbundling. // Check binary bundle/unbundle. The content that we have before bundling must be the same we have after unbundling.
Context not available.