[flang] Adjust _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT for Solaris

[rorth]

Even after 13e2200 (Solaris lacks femode_t, too), the Solaris flang build is still broken:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:87:5: error: static assertion failed due to requirement 'sizeof(fenv_t) <= sizeof(int) * 8': increase ieee_status_type size
   87 |     sizeof(fenv_t) <= sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT,      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:87:20: note: expression evaluates to '200 <= 32'
   87 |     sizeof(fenv_t) <= sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT,      |     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This patch fixes this by adjusting _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT accordingly.

Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

[llvmbot]

@llvm/pr-subscribers-flang-runtime

Author: Rainer Orth (rorth)

Changes

Even after 13e2200 (Solaris lacks femode_t, too), the Solaris flang build is still broken:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:87:5: error: static assertion failed due to requirement 'sizeof(fenv_t) <= sizeof(int) * 8': increase ieee_status_type size
   87 |     sizeof(fenv_t) <= sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT,      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:87:20: note: expression evaluates to '200 <= 32'
   87 |     sizeof(fenv_t) <= sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT,      |     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This patch fixes this by adjusting _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT accordingly.

Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

---

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

1 Files Affected:

• (modified) flang/include/flang/Runtime/magic-numbers.h (+1-1)

diff --git a/flang/include/flang/Runtime/magic-numbers.h b/flang/include/flang/Runtime/magic-numbers.h
index d00d5027d4ed2..cebf9f3a65478 100644
--- a/flang/include/flang/Runtime/magic-numbers.h
+++ b/flang/include/flang/Runtime/magic-numbers.h
@@ -107,6 +107,6 @@ as int arrays with the following extents to allow build time validation of
 these sizes in cross compilation environments.
 #endif
 #define _FORTRAN_RUNTIME_IEEE_FEMODE_T_EXTENT 2
-#define _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT 8
+#define _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT 50
 
 #endif

[vdonaldson]

@rorth Thanks for addressing this problem.

If the solaris sizeof(fenv_t) is actually 50*4=200 bytes, that is larger than other systems. This value increase will affect memory use for all systems.

Does removing this assert in exceptions.cpp resolve the solaris build problem?

static_assert(
    sizeof(fenv_t) <= sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT,
    "increase ieee_status_type size");

If yes, that would be a better choice for now.

[rorth]

Sorry for the delay in replying: github notifications somehow don't work for me.

Just removing the assertion fixes the Solaris build indeed: the testsuite results are identical to those with my patch.

How should we do this properly: just add a comment in exceptions.cpp or also add one in magic-numbers.h indicating that _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT is too small on Solaris?

[vdonaldson]

Just removing the assertion fixes the Solaris build indeed: the testsuite results are identical to those with my patch.

How should we do this properly: just add a comment in exceptions.cpp or also add one in magic-numbers.h indicating that _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT is too small on Solaris?

There is already a comment at the end of exceptions.cpp for femode_t objects, which is very similar to this problem. My preference is to add/extend the comment there.

My original code for this was an assert in the compiler rather than the runtime. That nicely solves all existing issues. The problem is that we would eventually like to support cross compilation, where a front end assert doesn't work. One way to resolve this would be to allocate these objects dynamically at runtime, although that would penalize typical compilations. Eventually we might want to generate different code for local compilation vs. cross compilation.

Bottom line - I'm still looking at this. For now I can live with something that allows builds to succeed.

[vdonaldson]

I can't tell from the build logs what the problem is with the solaris build invocation of lit test ieee_festatus.f90. Depending on what that problem is, you could either fix it, or disable the test for solaris if that is possible.

[rorth]

Just removing the assertion fixes the Solaris build indeed: the testsuite results are identical to those with my patch.
How should we do this properly: just add a comment in exceptions.cpp or also add one in magic-numbers.h indicating that _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT is too small on Solaris?

There is already a comment at the end of exceptions.cpp for femode_t objects, which is very similar to this problem. My preference is to add/extend the comment there.

I've updated the patch accordingly.

My original code for this was an assert in the compiler rather than the runtime. That nicely solves all existing issues. The problem is that we would eventually like to support cross compilation, where a front end assert doesn't work. One way to resolve this would be to allocate these objects dynamically at runtime, although that would penalize typical compilations. Eventually we might want to generate different code for local compilation vs. cross compilation.

Ah, understood. Coming from GCC where the target is fixed at compiler build time, I keep forgetting that LLVM is different in this regard.

Bottom line - I'm still looking at this. For now I can live with something that allows builds to succeed.

The revised patch did just that: re-tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

[rorth]

I can't tell from the build logs what the problem is with the solaris build invocation of lit test ieee_festatus.f90. Depending on what that problem is, you could either fix it, or disable the test for solaris if that is possible.

I had only seen that failure the first time I built with the femode_t patch included. In all later builts it PASSes just fine, so nothing to worry about.

[vdonaldson]

Thanks for working through this build issue. I'll look for a longer term fix.

Thanks.