sandialabs / seacas

The Sandia Engineering Analysis Code Access System (SEACAS) is a suite of preprocessing, postprocessing, translation, and utility applications supporting finite element analysis software using the Exodus database file format.
Other
132 stars 79 forks source link

static assert fires in exmemy.c on win32 build #405

Open Keno opened 11 months ago

Keno commented 11 months ago

I'm working on building Trilinos (including seacas) for distribution in the Julia language package ecosystem. Windows is one of our Tier 1 platforms, so I'm trying to build for windows as well (even though I don't expect may users).

I'm seeing the following assert fire in my 32-bit windows build:

https://github.com/sandialabs/seacas/blob/1e5138cf4b6a2855e54ea69b172df5ae36013a2b/packages/seacas/libraries/supes/ext_lib/exmemy.c#L112

Now, looking at https://github.com/sandialabs/seacas/blob/1e5138cf4b6a2855e54ea69b172df5ae36013a2b/packages/seacas/libraries/supes/fortranc.h#L14-L27

I'm assuming that FTNINT stands for fortran int and I see that at

https://github.com/sandialabs/seacas/blob/1e5138cf4b6a2855e54ea69b172df5ae36013a2b/packages/seacas/cmake/FortranSettings.cmake#L3C1-L18

the fortran integer size is unconditionally set to 8 bytes, which matches the definition of FTNINT, but obviously does not match the assert. Interestingly, it does build on linux32, because long int is 32 bits there (though it then of course does not match the fortran int size).

So my question is, what's the expected behavior:

  1. Should FTNINT always match the fortran int size (if so, is there an issue on linux32)?
  2. If so, does that mean that a 32bit build of seacas is unsupported? or
  3. Is there some other adjustment I should be doing.
gsjaardema commented 11 months ago

The 32-bit builds have definitely not been used lately and could probably be considered unsupported. The code still exists in various places, but I can't remember the last time I or someone did a 32-bit build especially with the fortran codes.

Depending on what you are trying to support, you might not need the fortran-based seacas applications. The exodus and IOSS libraries do not need fortran. There are a few seacas applications that are still commonly used that need fortran (explore, grepos are probably the two most used fortran seacas codes).

The whole fortran/C 32/64 bit integer strangeness started a few decades ago and we would definitely do something different if we had to start over today (and we have improved it somewhat...).

But, we do require that the size of the integers used by the fortran compiler are able to store a pointer since this is used in the dynamic memory system that we developed a long long time ago for use in fortran... I don't know enough about windows fortran to know how to force 8-byte integers in windows fortran, but the file you link to is how it is done on various compilers...

I can give more help if needed depending on your needs with this. Easiest may be to not build the fortran-dependent seacas codes which might give you the functionality you need.

Keno commented 11 months ago

The 32-bit builds have definitely not been used lately and could probably be considered unsupported. The code still exists in various places, but I can't remember the last time I or someone did a 32-bit build especially with the fortran codes.

I probably don't have users for this, I just need to make choice one way or another whether I should try to fix something (letting anybody who might care try at their own risk) or just disable it entirely, so I figured I'd check in.

But, we do require that the size of the integers used by the fortran compiler are able to store a pointer since this is used in the dynamic memory system that we developed a long long time ago for use in fortran...

So is the only requirement then that sizeof(FTNINT) >= sizeof(void *)? Just to clarify, FTNINT is properly 8 byte on 32bit windows with the current code, which matches Fortran as defined. The only issue is that sizeof(void*) is 4 byte, so the assert fails (and as mentioned FTNINT is likely incorrect on 32-bit linux).

I don't know enough about windows fortran to know how to force 8-byte integers in windows fortran, but the file you link to is how it is done on various compilers...

Our toolchain is just gfortran there, so the existing setting is working (which is what's causing the assert to fail ;) ).

gsjaardema commented 11 months ago

It might be OK to modify the test to sizeof(FTNINT) >= sizeof(void*). It's just been so long since I looked at the special cases in that code that I don't know for sure whether there are any assumptions for strict equality vs just being big enough.

I would think that if you make the change, it should show up immediately when running either explore or grepos whether anything is broken...