[flang] avoid GCC < 8 compiler failure after D80794

[clementval]

Summary

Patch D80794 remove the custom flags for release build for flang.
This leads to build failure with GCC < 8. This patch add upperbound check
in order to avoid the -Werror=array-bounds to trigger a build failure.

/home/4vn/versioning/llvm-project/flang/lib/Decimal/big-radix-floating-point.h:183:29: error: array subscript is above array bounds [-Werror=array-bounds]
           digit_[j] = digit_[j + remove];
                       ~~~~~~^
/home/4vn/versioning/llvm-project/flang/lib/Decimal/big-radix-floating-point.h:183:29: error: array subscript is above array bounds [-Werror=array-bounds]
           digit_[j] = digit_[j + remove];
                       ~~~~~~^
/home/4vn/versioning/llvm-project/flang/lib/Decimal/big-radix-floating-point.h:183:29: error: array subscript is above array bounds [-Werror=array-bounds]
           digit_[j] = digit_[j + remove];
                       ~~~~~~^
/home/4vn/versioning/llvm-project/flang/lib/Decimal/big-radix-floating-point.h:183:29: error: array subscript is above array bounds [-Werror=array-bounds]
           digit_[j] = digit_[j + remove];
                       ~~~~~~^
/home/4vn/versioning/llvm-project/flang/lib/Decimal/big-radix-floating-point.h:183:29: error: array subscript is above array bounds [-Werror=array-bounds]
           digit_[j] = digit_[j + remove];

/home/4vn/versioning/llvm-project/flang/include/flang/Evaluate/integer.h:809:28: error: array subscript is above array bounds [-Werror=array-bounds]
               xy += product[to];
                     ~~~~~~~^
/home/4vn/versioning/llvm-project/flang/include/flang/Evaluate/integer.h:810:22: error: array subscript is above array bounds [-Werror=array-bounds]
               product[to] = xy & partMask;
               ~~~~~~~^
/home/4vn/versioning/llvm-project/flang/include/flang/Evaluate/integer.h:809:28: error: array subscript is above array bounds [-Werror=array-bounds]
               xy += product[to];
                     ~~~~~~~^

[DavidTruby]

This looks good to me.

As an aside, I think this perfectly demonstrates why we should not force on Werror or have it on by default. We don't have public build bots for even the compilers we claim to support and seemingly innocent changes can cause spurious build failures for other users simply because we are forcing Werror on. And worse, at the moment we are forcing Werror on with no way to turn it off meaning these spurious errors can't even be avoided. This comment isn't intended in any way to block this patch; I just wanted to note it here.

[clementval]

In D81179#2074261, @DavidTruby wrote:

This looks good to me.

As an aside, I think this perfectly demonstrates why we should not force on Werror or have it on by default. We don't have public build bots for even the compilers we claim to support and seemingly innocent changes can cause spurious build failures for other users simply because we are forcing Werror on. And worse, at the moment we are forcing Werror on with no way to turn it off meaning these spurious errors can't even be avoided. This comment isn't intended in any way to block this patch; I just wanted to note it here.

Fully agree with your point on Werror. Thanks for the review.

[klausler]

If this is a version-specific work around for a g++ bug, please don't leave it enabled for later fixed versions of g++. These loops can be executed as part of formatted floating-point I/O and the performance cost of your extra per-iteration overhead may affect application runtime performance.

[clementval]

In D81179#2074420, @klausler wrote:

If this is a version-specific work around for a g++ bug, please don't leave it enabled for later fixed versions of g++. These loops can be executed as part of formatted floating-point I/O and the performance cost of your extra per-iteration overhead may affect application runtime performance.

Sure I'll send a patch to have the workaround only for GCC < 8.

[dblaikie]

If there are bugs in compiler warnings - probably disable the warning in the build system (I think we have a few such warning disablings), if the workaround is particularly awkward. (if there are nice rephrasing of the same code hat don't need to be commented but just look good/correct/fine/normal way to write the code, then I'd be willing to overlook some warning bugs like that)

flang/include/flang/Evaluate/integer.h

public:
   constexpr Product MultiplyUnsigned(const Integer &y) const {
     Part product[2 * parts]{}; // little-endian full product
     for (int j{0}; j < parts; ++j) {
       if (Part xpart{LEPart(j)}) {
         for (int k{0}; k < parts; ++k) {
           if (Part ypart{y.LEPart(k)}) {
             BigPart xy{xpart};
             xy *= ypart;
-            for (int to{j + k}; xy != 0; ++to) {
+            // && to < (2 * parts) was added to avoid GCC < 8 build failure
+            // on -Werror=array-bounds
+            for (int to{ j + k }; xy != 0 && to < (2 * parts); ++to) {
               xy += product[to];
               product[to] = xy & partMask;
               xy >>= partBits;
             }
           }
         }
       }
     }

flang/lib/Decimal/big-radix-floating-point.h

int RemoveLeastOrderZeroDigits() {
     int remove{0};
     if (digits_ > 0 && digit_[0] == 0) {
       while (remove < digits_ && digit_[remove] == 0) {
         ++remove;
       }
       if (remove >= digits_) {
         digits_ = 0;
       } else if (remove > 0) {
-        for (int j{0}; j + remove < digits_; ++j) {
+        // (&& j + remove < maxDigits) was added to avoid GCC < 8 build failure
+        // on -Werror=array-bounds
+        for (int j{ 0 }; j + remove < digits_ && (j + remove < maxDigits);
+             ++j) {
           digit_[j] = digit_[j + remove];
         }
         digits_ -= remove;
       }
     }
     return remove;
   }