[Flang][Runtime] Fix implicit conversion warning when targeting 32bit…

[serge-sans-paille]

… architecture

On 32 bit systems, TypeParameterValue is 64bit wide while CFI_index_t is 32bit wide.

[llvmbot]

@llvm/pr-subscribers-flang-runtime

Author: None (serge-sans-paille)

Changes

… architecture

On 32 bit systems, TypeParameterValue is 64bit wide while CFI_index_t is 32bit wide.

---

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

1 Files Affected:

• (modified) flang/runtime/derived.cpp (+17)

diff --git a/flang/runtime/derived.cpp b/flang/runtime/derived.cpp
index 0d9e033df4e27..e14aa2e927bde 100644
--- a/flang/runtime/derived.cpp
+++ b/flang/runtime/derived.cpp
@@ -90,8 +90,25 @@ RT_API_ATTRS int Initialize(const Descriptor &instance,
         comp.derivedType() && !comp.derivedType()->noInitializationNeeded()) {
       // Default initialization of non-pointer non-allocatable/automatic
       // data component.  Handles parent component's elements.  Recursive.
+<<<<<<< Updated upstream
       SubscriptValue extents[maxRank];
       GetComponentExtents(extents, comp, instance);
+=======
+      SubscriptValue extent[maxRank];
+      const typeInfo::Value *bounds{comp.bounds()};
+      for (int dim{0}; dim < comp.rank(); ++dim) {
+        auto lb = bounds[2 * dim].GetValue(&instance).value_or(0);
+        auto ub = bounds[2 * dim + 1].GetValue(&instance).value_or(0);
+        if (ub >= lb) {
+          auto bound_diff = ub - lb;
+          assert(bound_diff <= std::numeric_limits<SubscriptValue>::max() &&
+              "array bound overflow");
+          extent[dim] = bound_diff;
+        } else {
+          extent[dim] = 0;
+        }
+      }
+>>>>>>> Stashed changes
       StaticDescriptor<maxRank, true, 0> staticDescriptor;
       Descriptor &compDesc{staticDescriptor.descriptor()};
       const typeInfo::DerivedType &compType{*comp.derivedType()};

[jeanPerier]

Thanks for detecting the issue!

[jeanPerier]

Flang runtime code uses RUNTIME_CHECK macro to control how asserts are emitted inside the runtime.
I am not sure if the check should be in the runtime though. @klausler:

• Does the frontend allow constructing array component whose extents do not fit in SubscriptValue (I guess with PDTs that may be hard to enforce in semantics)?
• Is it intended for SubscriptValue to be 32bit while TypeParameterValue is 64bit on 32bit machine?

[klausler]

There's no consideration for 32-bit support in the frontend as far as I know, and it would be quite a project to try to add it.

[serge-sans-paille]

I assume that for a 32 bit target, we should never meet an overflow in subscript computation. But we could imagine targeting a 64 bit target from a 32 bit target (although flang doesn't compile from 32 bit arch at the moment), I guess that's why TypeParameterValue is 64 bit wide (?)

[serge-sans-paille]

A few words on the motivation : there are a few projects around there that compiles Lapack to wasm through a modified flang, e.g. https://github.com/r-wasm/flang-wasm . I'm trying to upstream the parts that could be relevant / beneficial to flang as a whole, without being too intrusive.

[serge-sans-paille]

I've updated the commit description and moved to a static_cast to reflect my understanding.

[jeanPerier]

I assume that for a 32 bit target, we should never meet an overflow in subscript computation.

It seems a correct assumption to me, SubscriptValue should just be 64bit on this target if one intended to support large susbcripts on 32bit target. Although as @klausler mentioned 32bit targets have not been tested by us.

A few words on the motivation : there are a few projects around there that compiles Lapack to wasm through a modified flang, e.g. https://github.com/r-wasm/flang-wasm . I'm trying to upstream the parts that could be relevant / beneficial to flang as a whole, without being too intrusive.

Thanks for that.

[jeanPerier]

nit: use braces init
In the runtime the style follows flang front-end coding style: https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md

The whole point of using braces init is to get warning when narrowing like it was the case so that we can asses and discuss these situation.

[jeanPerier]

I think the +1 was dropped by mistake. I advise going back to the ternary and put the static_cast in around ub - lb + 1. It is easier to read for me at least.

[serge-sans-paille]

Update done. There's indeed a mention of static_cast in the guide you linked.

[jeanPerier]

Thanks, LGTM