Skip to content

[Flang][OpenMP] Skip implicit mapping of named constants #145966

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

Merged
merged 1 commit into from
Jun 27, 2025

Conversation

TIFitis
Copy link
Member

@TIFitis TIFitis commented Jun 26, 2025

Added early return when mapping named constants.

This prevents linking error in the following example:

program test
   use, intrinsic :: iso_c_binding, only: c_double
   implicit none

   real(c_double) :: x
   integer        :: i
   x = 0.0_c_double
   !$omp target teams distribute parallel do reduction(+:x)
   do i = 0, 9
      x = x + 1.0_c_double
   end do
   !$omp end target teams distribute parallel do
end program test

Added early return when mapping implicit variables for named constants.

This prevents linking error in the following example:

program test
   use, intrinsic :: iso_c_binding, only: c_double
   implicit none

   real(c_double) :: x
   integer        :: i
   x = 0.0_c_double
   !$omp target teams distribute parallel do reduction(+:x)
   do i = 0, 9
      x = x + 1.0_c_double
   end do
   !$omp end target teams distribute parallel do
end program test
@TIFitis TIFitis requested review from skatrak, kparzysz and Copilot June 26, 2025 20:45
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jun 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a linking error by skipping the implicit mapping of named constants in OpenMP target regions. The key changes include adding an early return for constants and updating the inline comment to explain the behavior.

Comments suppressed due to low confidence (1)

flang/lib/Lower/OpenMP/OpenMP.cpp:2398

  • The comment mentions both parameters and constants, but the subsequent condition only checks for named constants. If parameters also require an early return, consider either updating the condition or adjusting the comment for clarity.
    // Skip parameters/constants as they do not need to be mapped.

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Akash Banerjee (TIFitis)

Changes

Added early return when mapping named constants.

This prevents linking error in the following example:

program test
   use, intrinsic :: iso_c_binding, only: c_double
   implicit none

   real(c_double) :: x
   integer        :: i
   x = 0.0_c_double
   !$omp target teams distribute parallel do reduction(+:x)
   do i = 0, 9
      x = x + 1.0_c_double
   end do
   !$omp end target teams distribute parallel do
end program test

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

1 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+4)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index ebd1d038716e4..b52818e5387a6 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2395,6 +2395,10 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
     if (dsp.getAllSymbolsToPrivatize().contains(&sym))
       return;
 
+    // Skip parameters/constants as they do not need to be mapped.
+    if (semantics::IsNamedConstant(sym))
+      return;
+
     // These symbols are mapped individually in processHasDeviceAddr.
     if (llvm::is_contained(hasDeviceAddrSyms, &sym))
       return;

Copy link
Member

@skatrak skatrak left a comment

Choose a reason for hiding this comment

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

Thanks Akash, LGTM!

@TIFitis TIFitis merged commit 91f10df into llvm:main Jun 27, 2025
10 checks passed
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
Added early return when mapping named constants.

This prevents linking error in the following example:

```
program test
   use, intrinsic :: iso_c_binding, only: c_double
   implicit none

   real(c_double) :: x
   integer        :: i
   x = 0.0_c_double
   !$omp target teams distribute parallel do reduction(+:x)
   do i = 0, 9
      x = x + 1.0_c_double
   end do
   !$omp end target teams distribute parallel do
end program test
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants