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

[flang] Put ISO_Fortran_binding.h where it can be easily used #70129

Merged
merged 3 commits into from
Oct 30, 2023

Conversation

psteinfeld
Copy link
Contributor

The update stems from the discussion in
https://discourse.llvm.org/t/adding-flang-specific-header-files-to-clang/72442

This is my third attempt at this. My second attempt was in pull request #69121.

This is my second attempt at this. My first attempt was in pull request #68756.

This pull request has three changes from the second one:

  • I put the test into the Driver directory rather than Examples so that it would get run without require the define LLVM_BUILD_EXAMPLES.
  • When installing ISO_Fortran_binding.h, I changed the location where it was installed from.
  • I changed the test so that it would work when flang was built with shared libraries.

Here's the information from my previous attempts:

I decided to put ISO_Fortran_binding.h in a place where it would be accessible with the include: "#include<ISO_Fortran_binding.h>" rather than "#include<fortran/ISO_Fortran_binding.h>" because this is what gfortran implements.

Note that the file is also installed into ".../include/flang", so if a user wanted to access the file from a compiler other than clang, it would be available.

I added a test in ".../flang/test/Driver". To make the test work, I also needed to put ISO_Fortran_binding.h into the build area.

Although the flang project depends on clang, clang may not always be available in a flang build. For example, when building just the "check-flang" target, the "clang" executable may not be available at the time the new test gets run. To account for this, I made the test's script check for the existence of the "clang" executable. If "clang" is not available, it simply prints "PASS". If it is available, it fully builds and executes the test. On success, this will also print "PASS"

The update stems from the discussion in
https://discourse.llvm.org/t/adding-flang-specific-header-files-to-clang/72442

This is my third attempt at this.  My second attempt was in pull request

This is my second attempt at this.  My first attempt was in pull request

This pull request has three changes from the second one:
- I put the test into the Driver directory rather than Examples so that
it would get run without require the define LLVM_BUILD_EXAMPLES.
- When installing ISO_Fortran_binding.h, I changed the location where it
was installed from.
- I changed the test so that it would work when flang was built with
shared libraries.

Here's the information from my previous attempts:

I decided to put ISO_Fortran_binding.h in a place where it would be
accessible with the include: "#include<ISO_Fortran_binding.h>" rather
than "#include<fortran/ISO_Fortran_binding.h>" because this is what
gfortran implements.

Note that the file is also installed into ".../include/flang", so if a
user wanted to access the file from a compiler other than clang, it
would be available.

I added a test in ".../flang/test/Driver".  To make the test work, I also
needed to put ISO_Fortran_binding.h into the build area.

Although the flang project depends on clang, clang may not always be available
in a flang build.  For example, when building just the "check-flang" target,
the "clang" executable may not be available at the time the new test gets run.
To account for this, I made the test's script check for the existence of the
"clang" executable.  If "clang" is not available, it simply prints "PASS".  If
it is available, it fully builds and executes the test.  On success, this will
also print "PASS"
@llvmbot llvmbot added the flang Flang issues not falling into any other category label Oct 24, 2023
The update stems from the discussion in
https://discourse.llvm.org/t/adding-flang-specific-header-files-to-clang/72442

This is my third attempt at this.  My second attempt was in pull request

This is my second attempt at this.  My first attempt was in pull request

This pull request has three changes from the second one:
- I put the test into the Driver directory rather than Examples so that
it would get run without require the define LLVM_BUILD_EXAMPLES.
- When installing ISO_Fortran_binding.h, I changed the location where it
was installed from.
- I changed the test so that it would work when flang was built with
shared libraries.

Here's the information from my previous attempts:

I decided to put ISO_Fortran_binding.h in a place where it would be
accessible with the include: "#include<ISO_Fortran_binding.h>" rather
than "#include<fortran/ISO_Fortran_binding.h>" because this is what
gfortran implements.

Note that the file is also installed into ".../include/flang", so if a
user wanted to access the file from a compiler other than clang, it
would be available.

I added a test in ".../flang/test/Driver".  To make the test work, I also
needed to put ISO_Fortran_binding.h into the build area.

Although the flang project depends on clang, clang may not always be available
in a flang build.  For example, when building just the "check-flang" target,
the "clang" executable may not be available at the time the new test gets run.
To account for this, I made the test's script check for the existence of the
"clang" executable.  If "clang" is not available, it simply prints "PASS".  If
it is available, it fully builds and executes the test.  On success, this will
also print "PASS"
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-flang-driver

Author: Pete Steinfeld (psteinfeld)

Changes

The update stems from the discussion in
https://discourse.llvm.org/t/adding-flang-specific-header-files-to-clang/72442

This is my third attempt at this. My second attempt was in pull request #69121.

This is my second attempt at this. My first attempt was in pull request #68756.

This pull request has three changes from the second one:

  • I put the test into the Driver directory rather than Examples so that it would get run without require the define LLVM_BUILD_EXAMPLES.
  • When installing ISO_Fortran_binding.h, I changed the location where it was installed from.
  • I changed the test so that it would work when flang was built with shared libraries.

Here's the information from my previous attempts:

I decided to put ISO_Fortran_binding.h in a place where it would be accessible with the include: "#include<ISO_Fortran_binding.h>" rather than "#include<fortran/ISO_Fortran_binding.h>" because this is what gfortran implements.

Note that the file is also installed into ".../include/flang", so if a user wanted to access the file from a compiler other than clang, it would be available.

I added a test in ".../flang/test/Driver". To make the test work, I also needed to put ISO_Fortran_binding.h into the build area.

Although the flang project depends on clang, clang may not always be available in a flang build. For example, when building just the "check-flang" target, the "clang" executable may not be available at the time the new test gets run. To account for this, I made the test's script check for the existence of the "clang" executable. If "clang" is not available, it simply prints "PASS". If it is available, it fully builds and executes the test. On success, this will also print "PASS"


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

2 Files Affected:

  • (modified) flang/CMakeLists.txt (+17)
  • (added) flang/test/Driver/ctofortran.f90 (+78)
diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt
index ac30da89995ed31..f81d3e33fe86c0c 100644
--- a/flang/CMakeLists.txt
+++ b/flang/CMakeLists.txt
@@ -276,6 +276,9 @@ endif()
 
 
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
+if (NOT PACKAGE_VERSION)
+  set(PACKAGE_VERSION ${LLVM_VERSION_MAJOR})
+endif()
 
 
 if (NOT DEFINED FLANG_VERSION_MAJOR)
@@ -490,3 +493,17 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
     PATTERN "*.inc"
     )
 endif()
+
+# Put ISO_Fortran_binding.h into the include files of the build area now
+# so that we can run tests before installing
+include(GetClangResourceDir)
+get_clang_resource_dir(HEADER_BINARY_DIR PREFIX ${LLVM_LIBRARY_OUTPUT_INTDIR}/.. SUBDIR include)
+configure_file(
+  ${FLANG_SOURCE_DIR}/include/flang/ISO_Fortran_binding.h
+  ${HEADER_BINARY_DIR}/ISO_Fortran_binding.h)
+
+# And also install it into the install area
+get_clang_resource_dir(HEADER_INSTALL_DIR SUBDIR include)
+install(
+  FILES include/flang/ISO_Fortran_binding.h
+  DESTINATION ${HEADER_INSTALL_DIR} )
diff --git a/flang/test/Driver/ctofortran.f90 b/flang/test/Driver/ctofortran.f90
new file mode 100644
index 000000000000000..402ece9d387be6c
--- /dev/null
+++ b/flang/test/Driver/ctofortran.f90
@@ -0,0 +1,78 @@
+! UNSUPPORTED: system-windows
+! RUN: split-file %s %t
+! RUN: chmod +x %t/runtest.sh
+! RUN: %t/runtest.sh %t %flang $t/ffile.f90 $t/cfile.c
+
+!--- ffile.f90
+subroutine foo(a) bind(c)
+  integer :: a(:)
+  if (lbound(a, 1) .ne. 1) then
+     print *, 'FAIL expected 1 for lbound but got ',lbound(a, 1)
+     stop 1
+  endif
+
+  if (ubound(a, 1) .ne. 10) then
+     print *, 'FAIL expected 10 for ubound but got ',ubound(a, 1)
+     stop 1
+  endif
+
+  do i = lbound(a,1),ubound(a,1)
+     !print *, a(i)
+     if (a(i) .ne. i) then
+        print *, 'FAIL expected', i, ' for index ',i, ' but got ',a(i)
+        stop 1
+     endif
+  enddo
+  print *, 'PASS'
+end subroutine foo
+
+! CHECK: PASS
+!--- cfile.c
+#include <stdio.h>
+#include <stdlib.h>
+#include <ISO_Fortran_binding.h>
+
+void foo(CFI_cdesc_t*);
+
+int a[10];
+
+int main() {
+  int i, res;
+  static CFI_CDESC_T(1) r1;
+  CFI_cdesc_t *desc = (CFI_cdesc_t*)&r1;
+  CFI_index_t extent[1] = {10};
+
+  for(i=0; i<10; ++i) {
+    a[i] = i+1;
+  }
+
+  res = CFI_establish(desc, (void*)a, CFI_attribute_other, CFI_type_int32_t,
+                      sizeof(int), 1, extent);
+  if (res != 0) {
+    printf("FAIL CFI_establish returned %d instead of 0.\n",res);
+    exit(1);
+  }
+
+  foo(desc);
+  return 0;
+}
+!--- runtest.sh
+#!/bin/bash
+export BINDIR=`dirname $2`
+export CCOMP=$BINDIR/clang
+echo "CCOMP: $CCOMP" > /home/psteinfeld/log
+echo "BINDIR: $BINDIR" >> /home/psteinfeld/log
+if [ -x $CCOMP ]
+then
+  export LIBDIR=$BINDIR/../lib
+  export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$LIBDIR
+  $CCOMP -c $1/$4 -o $1/cfile.o
+  echo "1: $1" >> /home/psteinfeld/log
+  echo "2: $2" >> /home/psteinfeld/log
+  echo "3: $3" >> /home/psteinfeld/log
+  $2 $1/$3 $1/cfile.o -o $1/ctofortran
+  $1/ctofortran # should print "PASS"
+else
+  # No clang compiler, just pass by default
+  echo "PASS"
+fi

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.

Looks good to me.

Comment on lines 63 to 64
echo "CCOMP: $CCOMP" > /home/psteinfeld/log
echo "BINDIR: $BINDIR" >> /home/psteinfeld/log
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this hardcoding of your name a leftover (here and below)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yikes! Thanks for catching this.

Thanks for catching this, Kiran!
Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

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

LG.

@psteinfeld
Copy link
Contributor Author

LG.

Thanks for the review, Kiran!

@psteinfeld psteinfeld merged commit ddd2747 into llvm:main Oct 30, 2023
3 checks passed
@psteinfeld psteinfeld deleted the ps-iso-fortran-binding branch October 30, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants