Skip to content

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 Copilot, kparzysz and skatrak 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 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
@TIFitis TIFitis deleted the iso_double branch July 2, 2025 14:10
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