util_nwchem_srcdir.F may have incorrect use of iso C bindings

[naromero77]

Here is the error that new flang gives me:

util_nwchem_srcdir.F:32:7: error: Whole scalar actual argument may not be associated with a dummy argument 'string=' array
        call utilc_nwchem_srcdir(compiled_name,length)

This seems very similar to the problem that I had with BerkeleyGW. https://github.com/flang-compiler/f18/issues/1074

I need to go offline for a couple of hours, but maybe one of you @edoapra @jeffhammond sees the problem.

[jeffhammond]

Personally, I think we should just rewrite the long path stuff with free-source form Fortran so we can skip all the hacks, but I don't have time to write it in the near future :disappointed:

[jeffhammond]

@naromero77 can you try this?

diff --git a/src/util/util_nwchem_srcdir.F b/src/util/util_nwchem_srcdir.F
index aef91064e9..788d4a0c49 100644
--- a/src/util/util_nwchem_srcdir.F
+++ b/src/util/util_nwchem_srcdir.F
@@ -18,6 +18,7 @@ C>
       USE ISO_C_BINDING
 #endif
       implicit none
+#include "util.fh"
 #ifdef NWCHEM_LONG_PATHS
 #include "utilc_nwchem_srcdir.fh"
       character (KIND=C_CHAR,LEN=256) :: compiled_name
@@ -26,7 +27,7 @@ C>
       character*256 compiled_name
       integer length
 #endif
-      character*(*) pathname !< [Output] The compiled in pathname
+      character*(nw_max_path_len) pathname !< [Output] The compiled in pathname
 #ifdef NWCHEM_LONG_PATHS
       length = len(compiled_name)
       call utilc_nwchem_srcdir(compiled_name,length)

[edoapra]

@naromero77 That patch does not work. One quick fix would be to disable the whole NWCHEM_LONG_PATHS define for f18, since it triggers all this iso_c bindings f18 behavior. If you disable NWCHEM_LONG_PATHS and revert to the vanilla f77-like source code, everything works. Does anybody know how the get the builtin pre-processor defines for f18?

[edoapra]

This ugly change seems to keep f18 happy, but I need to test it if it works at all

diff --git a/src/util/util_nwchem_srcdir.F b/src/util/util_nwchem_srcdir.F
index aef91064e9..c9e13a7c84 100644
--- a/src/util/util_nwchem_srcdir.F
+++ b/src/util/util_nwchem_srcdir.F
@@ -20,20 +20,25 @@ C>
       implicit none
 #ifdef NWCHEM_LONG_PATHS
 #include "utilc_nwchem_srcdir.fh"
-      character (KIND=C_CHAR,LEN=256) :: compiled_name
+      character (KIND=C_CHAR,LEN=1) :: compiled_name(256)
       integer (C_INT) :: length
+      character(len=*), intent(inout) :: pathname
+      integer j
 #else
       character*256 compiled_name
       integer length
-#endif
       character*(*) pathname !< [Output] The compiled in pathname
+#endif
 #ifdef NWCHEM_LONG_PATHS
       length = len(compiled_name)
       call utilc_nwchem_srcdir(compiled_name,length)
+      do j=1,length
+         pathname(j:j) = compiled_name(j)
+      enddo
 #else
       compiled_name =
      &NWCHEM_SRCDIR
-#endif
       pathname = compiled_name
+#endif
       end
 c $Id$

[jeffhammond]

Sorry about the bad patch.

[naromero77]

I can confirm that the new flang front-end is happy, but I have not tried to run it.

I also found the patch that was implemented for BerkeleyGW that had the same issue and the fix was similar.

I am not sure if this is necessary for the NWChem code, but for the BerkeleyGW code the author (not me) initialized the character arrays to c_null_char. Do we have to make sure strings are null terminated?

[edoapra]

@naromero77 I am not sure about the strings null termination. Let's see what I get when I manage to get a build going. In the meantime I am struggling to get a fix for the missing rshift and lshift functions ... is there any alternative?

[hjjvandam]

Hi Romero,

Initializing the whole string to all null characters seems a bit paranoid. The main thing is that inp_strlen has to return the correct length of the string value. This function looks for the first non-space character coming from the end. It does not expect null characters. So if you want to initialize the whole string initializing it to “ “ is probably best.

Best wishes,

 Huub

---

Hubertus van Dam, 631-344-6020, hvandam@bnl.govmailto:hvandam@bnl.gov Brookhaven National Laboratory

From: "Nichols A. Romero" notifications@github.com Reply-To: nwchemgit/nwchem reply@reply.github.com Date: Thursday, June 11, 2020 at 21:22 To: nwchemgit/nwchem nwchem@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [nwchemgit/nwchem] util_nwchem_srcdir.F may have incorrect use of iso C bindings (#212)

I can confirm that the new flang front-end is happy, but I have not tried to run it.

I also found the patch that was implemented for BerkeleyGW that had the same issue and the fix was similar.

I am not sure if this is necessary for the NWChem code, but for the BerkeleyGW code the author (not me) initialized the character arrays to c_null_char. Do we have to make sure strings are null terminated?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/nwchemgit/nwchem/issues/212#issuecomment-643013010, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABDS7HVCRUZOPR66QC6WS6TRWF7NHANCNFSM4N32W6OQ.

[naromero77]

@edoapra Can you clarify what is the problem with lshift and rshift? Sorry, I am not following.

[naromero77]

You mean the compiler issue in util_pack.F ?

[edoapra]

@naromero77 I have checked quite a few changes to address issues with the f18 parser. However, I am not able to get a working executable. My guess is that one of the problem is that f18 does not make correct use of the -i8 option. In other words, f18 with the -i8 flag does not see to promote integers to 64-bit integers

[naromero77]

@edoapra I am tied up in meetings most of this day, I will be online around 2pm central (hopefully).

Thank you for all the commits to address the f18 parsing issues. Most of us on the ECP Flang project are only trying to run Fortran code through the parser at the moment. I have not attempted to link and create a viable executable. I will ask about this -i8 option whether it is officially supported.

Just to clarify. There is only the f18 front-end at the moment for the new Flang. The flang team has not created the driver that takes the output from the front-end and passes it to the LLVM to create a linker. That is currently WIP. Right now the f18 tool passes the output to pgf90, but this can be changed by setting the F18_FC environment variable to something like gfortran

[edoapra]

@naromero77 Nick, I can confirm that -i8 does not work. This is a simple reproducer that works correctly with pg90 and gfortran, but it does not with f18. If -i8 does not work, it does make any sense to use f18 to compile NWChem.

      program main
      integer i
      write(6,*) ' size of i ',sizeof(i)
      stop
      end

$ f18 -i8 size.f

$ ./a.out
  size of i             4
FORTRAN STOP

$ pgf90 -i8 size.f

$ ./a.out
  size of i                         8
FORTRAN STOP

[jeffhammond]

@naromero77 How do we pass flags to the back-end compiler? As long as F18 doesn't translate INTEGER to a fixed-size INTEGER kind, then adding the -i8/-fdefault-integer-8 flag in the back-end compiler should work.

[naromero77]

@jeffhammond @edoapra There is no special magic to pass to the back-end compiler. It should just work. I think there is something broken and I will file a Bugzilla issue once I test it out myself shortly.

For reference, here is the line that should just take care of it: https://github.com/llvm/llvm-project/blob/master/flang/tools/f18/f18.cpp#L535

[edoapra]

@jeffhammond @edoapra There is no special magic to pass to the back-end compiler. It should just work. I think there is something broken and I will file a Bugzilla issue once I test it out myself shortly.

For reference, here is the line that should just take care of it: https://github.com/llvm/llvm-project/blob/master/flang/tools/f18/f18.cpp#L535

Sure, it might well be that my f18/pgf90 environment is not setup correctly. I have tried F18_FC=gfortran, but I got the same result

[naromero77]

@edoapra It is not on your side. It is broken, but is supposed to work I think.

I started looking at the C++ code, but I could not figure it out.

[naromero77]

The LLVM bugzilla requires registration, but for reference bug is reported here: https://bugs.llvm.org/show_bug.cgi?id=46307

[naromero77]

We got a bit side tracked. But I am closing this issue, since the original problem has been resolved.