Skip to content
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.

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

[OpenMPIRBuilder] NFC: Improve description of IsTargetDevice and IsGPU #79322

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

skatrak
Copy link
Contributor

@skatrak skatrak commented Jan 24, 2024

This patch tries to better explain the differences between the IsTargetDevice and IsGPU flags of the OpenMPIRBuilderConfig.

This patch tries to better explain the differences between the `IsTargetDevice`
and `IsGPU` flags of the `OpenMPIRBuilderConfig`.
@llvmbot llvmbot added flang:openmp clang:openmp OpenMP related changes to Clang labels Jan 24, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 24, 2024

@llvm/pr-subscribers-flang-openmp

Author: Sergio Afonso (skatrak)

Changes

This patch tries to better explain the differences between the IsTargetDevice and IsGPU flags of the OpenMPIRBuilderConfig.


Full diff: https://github.com/llvm/llvm-project/pull/79322.diff

1 Files Affected:

  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+12-3)
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 669104307fa0e23..2288969ecc95c47 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -83,11 +83,20 @@ BasicBlock *splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch,
 /// are ones that are not dependent on the configuration.
 class OpenMPIRBuilderConfig {
 public:
-  /// Flag for specifying if the compilation is done for embedded device code
-  /// or host code.
+  /// Flag to define whether to generate code for the role of the OpenMP host
+  /// (if set to false) or device (if set to true) in an offloading context. It
+  /// is set when the -fopenmp-is-target-device compiler frontend option is
+  /// specified.
   std::optional<bool> IsTargetDevice;
 
-  /// Flag for specifying if the compilation is done for an accelerator.
+  /// Flag for specifying if the compilation is done for an accelerator. It is
+  /// set according to the architecture of the target triple and currently only
+  /// true when targeting AMDGPU or NVPTX. Today, these targets can only perform
+  /// the role of an OpenMP target device, so `IsTargetDevice` must also be true
+  /// if `IsGPU` is true. This restriction might be lifted if an accelerator-
+  /// like target with the ability to work as the OpenMP host is added, or if
+  /// the capabilities of the currently supported GPU architectures are
+  /// expanded.
   std::optional<bool> IsGPU;
 
   // Flag for specifying if offloading is mandatory.

Copy link
Contributor

@DominikAdamski DominikAdamski left a comment

Choose a reason for hiding this comment

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

LGTM

@skatrak
Copy link
Contributor Author

skatrak commented Jan 29, 2024

Thanks Dominik for giving it a look. I'll wait one more day before merging, in case there are any further suggestions.

@skatrak skatrak merged commit fdac7d0 into llvm:main Jan 30, 2024
5 of 6 checks passed
@skatrak skatrak deleted the nfc-is-gpu-target-device-desc branch January 30, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants