Modify

Opened 4 years ago

Last modified 4 years ago

#1591 new defect

Simfactory: add -Wall and '-warn all' options to warning flags for intel compilers

Reported by: bmundim Owned by: Erik Schnetter
Priority: major Milestone:
Component: SimFactory Version: development version
Keywords: Cc:

Description

If you add the flag '-warn all' to F77_WARN_FLAGS and F90_WARN_FLAGS the compiler catches a subtle error (compiler error #8284) related to subroutine and function calls in Fortran:

https://software.intel.com/en-us/forums/topic/342615
https://software.intel.com/en-us/blogs/2009/03/31/doctor-fortran-in-ive-come-here-for-an-argument

Currently grhydro won't compile if we turn on that flag. I suggest to update all configurations in simfactory using intel compilers with the flag mentioned above and fix any issue related to this bug on the ET thorns. Any thoughts on my
proposal?

Attachments (0)

Change History (8)

comment:1 Changed 4 years ago by Erik Schnetter

I am not aware of an actual problem that this standard violation causes. (Having looked at the ABI and calling conventions, i.e. how the arguments are passed in machine code, I believe that our current code is currently safe on all the architectures we currently use, even if we violate the Fortran standard.) In saying this, I assume that this is about passing straight scalar, and not using interfaces to pass allocatable variables, pointers, optional arguments, etc. Please correct me if this assumption is wrong.

I thus suggest to release as-is, and to add the warning flags after the release, correcting the code then.

comment:2 Changed 4 years ago by Ian Hinder

If changing the warning flags might make some working code not compile, I don't think we should do it so soon before the release. I agree with Erik's proposal in comment:1.

comment:3 Changed 4 years ago by Ian Hinder

Milestone: ET_2014_05

comment:4 Changed 4 years ago by bmundim

Hi Erik,

I think you can pass allocatable variables, pointers, etc without using interfaces. What one needs to be careful is on matching the kind of object being passed as an actual argument to the dummy argument one. One might be lucky and nothing goes wrong, but that could lead to problems in the future. I would rather watch this issue closely.

In any case, I changed loewe.cfg so that grhydro would compile there (Added '-warn nointerfaces' option). After the release, please let me know which machines should I add the warning flags so I don't break compilation everywhere.

Thanks!

comment:5 Changed 4 years ago by Roland Haas

I did not quite follow the discussion but would like to. Bruno, would you mind to paste one of the actual error messages, please? Making a half-blind guess, are the errors occuring in connection with EOS calls where we actually declare the calls to take arrays as arguments but then often just pass a single scalar? I agree that this is in violation of the Fortran standard, so far it has not caused problems and since it is quite cumbersome to fix it everywhere it occurs so far we have not bothered. I'd also be very surprised if there actually is a Fortran compiler out there that passes a scalar A and a array A differently since basically all existing C interface code assumes that both are passed as a simple pointer.

comment:6 Changed 4 years ago by bmundim

Hi Roland,

here it is a sample of the error message:

/home/astro/mundim/tmp/einstein_test/Cactus/arrangements/EinsteinEvolve/GRHydro/src/GRHydro_Wrappers.F90(252): error #8284: If the actual argument is scalar, the dummy argument shall be scalar unless the actual argument is of type character or is an element of an array that is not assumed shape, pointer, or polymorphic.   [DDENS]
  call prim2conM(handle, gxx, gxy, gxz, gyy, gyz, gzz, sdetg, dens,&
-------^

the same error message applies for the following variables: [DSX], [DSY], [DSZ], [DTAU], [DBCONSX], [DBCONSY], [DBCONSZ], [DRHO], [DVELX], [DVELY], [DVELZ], [DEPS], [DPRESS],[DBVCX], [DBVCY], [DBVCZ].

One example of how not checking the argument passing might lead to a problem can be seen in this simple example from the site I mentioned above. Consider the following piece of code:

real a(10)
call sub(a(3))
...
subroutine sub (b)
real b(8)

if we declare b to be 'real b(10)' for example it might lead to problems when you try to access memory location associated with the elements b(9) and b(10). The compiler might or not detect this error. In case of the intel compiler it does detect this error if we set '-warn interfaces' or '-warn all' options.

comment:7 Changed 4 years ago by bmundim

Erik: out of curiosity, what were the tools you used to check the ABI and calling conventions? Could you give me a simple example?

Thanks,
Bruno.

comment:8 Changed 4 years ago by Erik Schnetter

Bruno, what I meant that I looked at the compiler documentation and the ABI documentation. From this I know that passing a scalar when the dummy argument is a one-element array is safe, even though the Fortran standard does not allow it.

I did not use a tool to check whether the code adheres to the Fortran standard. I believe the compiler when it says that the code violates the standard. In the long run, we may want to fix this.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain Erik Schnetter.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from Erik Schnetter to the specified user.
Next status will be 'confirmed'.
The owner will be changed from Erik Schnetter to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.