Skip to content

Conversation

@Renaud-K
Copy link
Contributor

I am trying again to get an intrinsic call to be intercepted in ... semantics? And once again, I am unable to get it to work.
The code is considerably simplified because I am really only interested to get genAtomicAdd to be called.
The test is

module atomictests
  contains
    attributes(global) subroutine testatomicdadd( a )
    real*8 :: a
    real*8 r,istat
    r = dble(threadIdx%x)
    istat = atomicaddd(a, r)
    return
    end subroutine testatomicdadd
end module atomictests

Can you see what I am missing?
Thank you

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jan 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 19, 2025

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

Author: Renaud Kauffmann (Renaud-K)

Changes

I am trying again to get an intrinsic call to be intercepted in ... semantics? And once again, I am unable to get it to work.
The code is considerably simplified because I am really only interested to get genAtomicAdd to be called.
The test is

module atomictests
  contains
    attributes(global) subroutine testatomicdadd( a )
    real*8 :: a
    real*8 r,istat
    r = dble(threadIdx%x)
    istat = atomicaddd(a, r)
    return
    end subroutine testatomicdadd
end module atomictests

Can you see what I am missing?
Thank you


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

3 Files Affected:

  • (modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+1)
  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+10)
  • (modified) flang/module/cudadevice.f90 (+34-1)
diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index 9c9c0609f4fc3c..e2ea89483ef11f 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -185,6 +185,7 @@ struct IntrinsicLibrary {
   mlir::Value genAnint(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genAny(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genAtanpi(mlir::Type, llvm::ArrayRef<mlir::Value>);
+  mlir::Value genAtomicAdd(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue
       genCommandArgumentCount(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genAsind(mlir::Type, llvm::ArrayRef<mlir::Value>);
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 6a343645ab8786..8aa21fbea8fd75 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -147,6 +147,7 @@ static constexpr IntrinsicHandler handlers[]{
     {"atan2pi", &I::genAtanpi},
     {"atand", &I::genAtand},
     {"atanpi", &I::genAtanpi},
+    {"atomicaddd", &I::genAtomicAdd, {}, /*isElemental=*/false},
     {"bessel_jn",
      &I::genBesselJn,
      {{{"n1", asValue}, {"n2", asValue}, {"x", asValue}}},
@@ -2574,6 +2575,15 @@ mlir::Value IntrinsicLibrary::genAtanpi(mlir::Type resultType,
   return builder.create<mlir::arith::MulFOp>(loc, atan, factor);
 }
 
+mlir::Value IntrinsicLibrary::genAtomicAdd(mlir::Type resultType,
+                                           llvm::ArrayRef<mlir::Value> args) {
+  assert(args.size() == 2);
+  llvm::errs() << "In genAtomicAdd\n";
+  return builder.create<mlir::LLVM::AtomicRMWOp>(
+      loc, mlir::LLVM::AtomicBinOp::add, args[0], args[1],
+      mlir::LLVM::AtomicOrdering::seq_cst);
+}
+
 // ASSOCIATED
 fir::ExtendedValue
 IntrinsicLibrary::genAssociated(mlir::Type resultType,
diff --git a/flang/module/cudadevice.f90 b/flang/module/cudadevice.f90
index 3d487fd000a094..e49ca4be69e5d2 100644
--- a/flang/module/cudadevice.f90
+++ b/flang/module/cudadevice.f90
@@ -92,5 +92,38 @@ attributes(device) subroutine threadfence_system()
     end function
   end interface
   public :: __fadd_ru
-  
+
+interface
+  attributes(device) real*8 function atomicaddd(address, val)
+  real*8, intent(inout) :: address
+  real*8 :: val
+  end function
+end interface
+public :: atomicaddd
+
+
+!interface atomicadd
+!  attributes(device) pure integer function atomicaddi(address, val)
+!!dir$ ignore_tkr (rd) address, (d) val
+!  integer, intent(inout) :: address
+!  integer, value :: val
+!  end function
+!  attributes(device) pure real function atomicaddf(address, val)
+!!dir$ ignore_tkr (rd) address, (d) val
+!  real, intent(inout) :: address
+!  real, value :: val
+!  end function
+!  attributes(device) pure real*8 function atomicaddd(address, val)
+!!dir$ ignore_tkr (rd) address, (d) val
+!  real*8, intent(inout) :: address
+!  real*8, value :: val
+!  end function
+!  attributes(device) pure integer(8) function atomicaddul(address, val)
+!!dir$ ignore_tkr (rd) address, (dk) val
+!  integer(8), intent(inout) :: address
+!  integer(8), value :: val
+!  end function
+!end interface 
+!public :: atomicadd
+
 end module

@nikic nikic changed the title WIP: attempting to add an atomicadd intrinsinc [flang] WIP: attempting to add an atomicadd intrinsinc Jan 19, 2025
@clementval
Copy link
Contributor

Have you tried without adding the interface? Other intrinsics don't have a Fortran interface defined in a module.

@Renaud-K
Copy link
Contributor Author

Wouldn't I need a body without an interface? Could you point me to these other intrinsics?

@clementval
Copy link
Contributor

Wouldn't I need a body without an interface? Could you point me to these other intrinsics?

It should not be declared as an interface or even as a subroutine. None of the intrinsics in the table have a definition in a fortran module file. If they had one, their name would be mangled and they would not match the one in the table.

@jeanPerier
Copy link
Contributor

jeanPerier commented Jan 20, 2025

Have you tried without adding the interface? Other intrinsics don't have a Fortran interface defined in a module.

Actually, intrinsic module procedure must have a Fortran interface defined an intrinsic module file because they are only recognized if those modules are used. The table can also be used for those procedures since this commit for ieee procedures.

@Renaud-K, in your test in the PR description, I do not see a use cudadevice, I think it is needed so that atomicaddd is resolved in semantics as being declared in the intrinsic module, and not being some user defined procedure.

[EDIT:] If you want to add it as an intrinsic and not a intrinsic module procedure (so that the user does not have to do some use xxx), you have two options:

  • Option 1, modifying lib/Evaluate/intrinsics.cpp to recognize that name as an intrinsic. This solution is meant for Fortran intrinsic, and generic extension, it has not really be used for intrinsics only valid in a certain contexts (e.g., there is no logic to only enable certain table entries when CUF, ACC, or OMP are enabled).
  • OPTION 2, still treat it as an intrinsic module procedure under the hood, but add an implicit use of that module in semantic based on some context (e.g., CUF). This is how PowerPC intrinsic, that work without the user proving any use ... on PowerPC were added, see this commit for an example.

@clementval
Copy link
Contributor

Actually, intrinsic module procedure must have a Fortran interface defined an intrinsic module file because they are only recognized if those modules are used. The table can also be used for those procedures since this commit for ieee

My bad ... I didn't remember about this.

@Renaud-K, in your test in the PR description, I do not see a use cudadevice, I think it is needed so that atomicaddd is resolved in semantics as being declared in the intrinsic module, and not being some user defined procedure.

cudadevice is implicitly "used" in cuda fortran device/global procedures.

@Renaud-K
Copy link
Contributor Author

Pardon me. This another one of our build issues. I simply forgot to relink f18.

@Renaud-K Renaud-K closed this Jan 21, 2025
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.

4 participants