Skip to content

[clang][Driver][HIP] Change OffloadingActionBuilder to respect the --no-gpu-bundle-output flag #163834

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −5
Open

[clang][Driver][HIP] Change OffloadingActionBuilder to respect the --no-gpu-bundle-output flag #163834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 11 additions & 5 deletions clang/lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3844,6 +3844,9 @@ class OffloadingActionBuilder final {
/// Flag set to true if all valid builders allow file bundling/unbundling.
bool CanUseBundler;

/// Flag set to false if an argument turns off bundling.
bool ShouldUseBundler;

public:
OffloadingActionBuilder(Compilation &C, DerivedArgList &Args,
const Driver::InputList &Inputs)
Expand Down Expand Up @@ -3878,6 +3881,9 @@ class OffloadingActionBuilder final {
}
CanUseBundler =
ValidBuilders && ValidBuilders == ValidBuildersSupportingBundling;

ShouldUseBundler = Args.hasFlag(options::OPT_gpu_bundle_output,
options::OPT_no_gpu_bundle_output, true);
}

~OffloadingActionBuilder() {
Expand Down Expand Up @@ -4029,11 +4035,11 @@ class OffloadingActionBuilder final {
SB->appendTopLevelActions(OffloadAL);
}

// If we can use the bundler, replace the host action by the bundling one in
// the resulting list. Otherwise, just append the device actions. For
// device only compilation, HostAction is a null pointer, therefore only do
// this when HostAction is not a null pointer.
if (CanUseBundler && HostAction &&
// If we can and should use the bundler, replace the host action by the
// bundling one in the resulting list. Otherwise, just append the device
// actions. For device only compilation, HostAction is a null pointer,
// therefore only do this when HostAction is not a null pointer.
if (CanUseBundler && ShouldUseBundler && HostAction &&
HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
// Add the host action to the list in order to create the bundling action.
OffloadAL.push_back(HostAction);
Expand Down
18 changes: 18 additions & 0 deletions clang/test/Driver/no-gpu-bundle-respected.hip
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// RUN: %clang -ccc-print-phases -c -emit-llvm \
// RUN: --offload-arch=gfx900,gfx1030 -O3 -x hip %s \
// RUN: 2>&1 | FileCheck %s --check-prefix=OFFLOAD

// RUN: %clang -ccc-print-phases -c -emit-llvm \
// RUN: --gpu-bundle-output --offload-arch=gfx900,gfx1030 -O3 -x hip %s \
// RUN: 2>&1 | FileCheck %s --check-prefix=OFFLOAD

// RUN: %clang -ccc-print-phases -c -emit-llvm \
// RUN: --no-gpu-bundle-output --offload-arch=gfx900,gfx1030 -O3 -x hip %s \
// RUN: 2>&1 | FileCheck %s --check-prefix=OFFLOAD2

// OFFLOAD: clang-offload-bundler
// OFFLOAD2-NOT: clang-offload-bundler
Copy link
Collaborator

Choose a reason for hiding this comment

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

use a meaningful check line, e.g. BUNDLE, NOBUNDLE. also check the action entries for the NOBUNDLE case.

Copy link
Author

Choose a reason for hiding this comment

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

use a meaningful check line

ah! so sorry, I should have caught those

also check the action entries for the NOBUNDLE case.

I tried looking through the -ccc-print-phases and the verbose outputs and I couldn't find a difference besides the order (which I thought might be flaky?) and the absence of the clang-offload-bundler command (which I'm already testing for). is there another way to test that which I'm missing?


int square(int num) {
return num * num;
}