Skip to content

[Flang][Runtime] Improve runtime implementation of the RENAME intrinsic #99445

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 2 commits into from
Jul 18, 2024

Conversation

mjklemm
Copy link
Contributor

@mjklemm mjklemm commented Jul 18, 2024

The RENAME implementation in the Fortran runtime had a few glitches that had to be addressed:

  • Wrong usage of RTDECL (fixed)
  • Issue fatal error when trying to use RENAME on a target device (fixed)

The RENAME implementation in the Fortran runtime had a few glitches that
had to be addressed:

- Wrong usage of RTDECL (fixed)
- Issue fatal error when trying to use RENAME on a target device (fixed)
@mjklemm mjklemm self-assigned this Jul 18, 2024
@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category labels Jul 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-flang-runtime

Author: Michael Klemm (mjklemm)

Changes

The RENAME implementation in the Fortran runtime had a few glitches that had to be addressed:

  • Wrong usage of RTDECL (fixed)
  • Issue fatal error when trying to use RENAME on a target device (fixed)

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

1 Files Affected:

  • (modified) flang/runtime/misc-intrinsic.cpp (+5-2)
diff --git a/flang/runtime/misc-intrinsic.cpp b/flang/runtime/misc-intrinsic.cpp
index 2f7fcd2e2341f..36264eca99d70 100644
--- a/flang/runtime/misc-intrinsic.cpp
+++ b/flang/runtime/misc-intrinsic.cpp
@@ -56,10 +56,10 @@ static RT_API_ATTRS void TransferImpl(Descriptor &result,
 extern "C" {
 RT_EXT_API_GROUP_BEGIN
 
-void RTDECL(Rename)(const Descriptor &path1, const Descriptor &path2,
+void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2,
     const Descriptor *status, const char *sourceFile, int line) {
   Terminator terminator{sourceFile, line};
-
+#if !defined(RT_DEVICE_COMPILATION)
   char *pathSrc{EnsureNullTerminated(
       path1.OffsetElement(), path1.ElementBytes(), terminator)};
   char *pathDst{EnsureNullTerminated(
@@ -84,6 +84,9 @@ void RTDECL(Rename)(const Descriptor &path1, const Descriptor &path2,
   if (pathDst != path2.OffsetElement()) {
     FreeMemory(pathDst);
   }
+#else  // !defined(RT_DEVICE_COMPILATION)
+  terminator.Crash("RENAME intrinsic is only supported on host devices");
+#endif // !defined(RT_DEVICE_COMPILATION)
 }
 
 void RTDEF(Transfer)(Descriptor &result, const Descriptor &source,

Copy link

github-actions bot commented Jul 18, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

Do all runtime functions that call POSIX/system stuff need to have this preprocessor check going forward or is there something special about this one/this file?

@mjklemm
Copy link
Contributor Author

mjklemm commented Jul 18, 2024

@vzakhari Pointed this out for the original PR for RENAME. I think that it is a valid point for functionality that is (not yet) available for device code.

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

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

Thank you for the follow-up.

@mjklemm mjklemm merged commit d06b55e into llvm:main Jul 18, 2024
7 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…ic (#99445)

The RENAME implementation in the Fortran runtime had a few glitches that
had to be addressed:

- Wrong usage of RTDECL (fixed)
- Issue fatal error when trying to use RENAME on a target device (fixed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:runtime flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants