On Tue, Mar 25, 2025 at 11:36:10PM -0500, Robert Dubner wrote:
> I took a minute to apply the patch and run the tests.  Ten of the UAT
> tests fail; they are the ones that test the ROUNDED clause.
> 
> It's 00:30 local time here, so I am not going to look into it now.  But
> here is a simple case so that you have something to chew on while I am
> getting my beauty sleep:
> 
>        IDENTIFICATION DIVISION.
>        PROGRAM-ID. prog.
>        DATA DIVISION.
>        WORKING-STORAGE SECTION.
>        01  N                PIC S9.
>        PROCEDURE DIVISION.
>            COMPUTE N ROUNDED MODE AWAY-FROM-ZERO = -2.51
>            DISPLAY "N should be -3"
>            DISPLAY "N        is " N
>            GOBACK.
>            END PROGRAM prog.
> 
>       N should be -3
>       N        is +1

Sorry, got the conversion wrong in 2 spots.

For signable the old code did
      if( value < 0 && (((pvalue[capacity-1] & 0x80) == 0 )))
        {
        capacity *= 2;
        }
      else if( value >= 0 && (((pvalue[capacity-1] & 0x80) == 0x80 )))
        {
        capacity *= 2;
        }
value < 0 has been correctly replaced with wi::neg_p (value)
and (pvalue[capacity-1] & 0x80) with
(value & wi::set_bit_in_zero<FIXED_WIDE_INT(128)>(capacity * 8 - 1))
(both are testing the same bit), but I got the two comparisons exactly
the other way, for negative values it was testing if that bit is set
(while previously it was testing if it was clear) and vice versa.

Here is a fixed patch, passes make check-cobol, but that is unfortunately
still not enough.

2025-03-26  Jakub Jelinek  <ja...@redhat.com>

        PR cobol/119242
        * cobol/genutil.h (get_power_of_ten): Remove #pragma GCC diagnostic
        around declaration.
        * cobol/genapi.cc (psa_FldLiteralN): Change type of value from
        __int128 to FIXED_WIDE_INT(128).  Remove #pragma GCC diagnostic
        around the declaration.  Use wi::min_precision to determine
        minimum unsigned precision of the value.  Use wi::neg_p instead
        of value < 0 tests and wi::set_bit_in_zero<FIXED_WIDE_INT(128)>
        to build sign bit.  Handle field->data.capacity == 16 like
        1, 2, 4 and 8, use wide_int_to_tree instead of build_int_cst.
        (mh_source_is_literalN): Remove #pragma GCC diagnostic around
        the definition.
        (binary_initial_from_float128): Likewise.
        * cobol/genutil.cc (get_power_of_ten): Remove #pragma GCC diagnostic
        before the definition.

--- gcc/cobol/genutil.h.jj      2025-03-25 21:14:48.448384925 +0100
+++ gcc/cobol/genutil.h 2025-03-25 21:19:24.358620134 +0100
@@ -104,10 +104,7 @@ void      get_binary_value( tree value,
 tree      get_data_address( cbl_field_t *field,
                             tree         offset);
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpedantic"
 FIXED_WIDE_INT(128) get_power_of_ten(int n);
-#pragma GCC diagnostic pop
 void      scale_by_power_of_ten_N(tree value,
                                 int N,
                                 bool check_for_fractional = false);
--- gcc/cobol/genapi.cc.jj      2025-03-25 21:11:06.767409766 +0100
+++ gcc/cobol/genapi.cc 2025-03-26 13:16:23.932827326 +0100
@@ -3798,16 +3798,13 @@ psa_FldLiteralN(struct cbl_field_t *fiel
   // We are constructing a completely static constant structure, based on the
   // text string in .initial
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpedantic"
-  __int128 value = 0;
-#pragma GCC diagnostic pop
+  FIXED_WIDE_INT(128) value = 0;
 
   do
     {
     // This is a false do{}while, to isolate the variables:
 
-    // We need to convert data.initial to an __int128 value
+    // We need to convert data.initial to an FIXED_WIDE_INT(128) value
     char *p = const_cast<char *>(field->data.initial);
     int sign = 1;
     if( *p == '-' )
@@ -3903,24 +3900,24 @@ psa_FldLiteralN(struct cbl_field_t *fiel
 
     // We now need to calculate the capacity.
 
-    unsigned char *pvalue = (unsigned char *)&value;
+    unsigned int min_prec = wi::min_precision(value, UNSIGNED);
     int capacity;
-    if( *(uint64_t*)(pvalue + 8) )
+    if( min_prec > 64 )
       {
       // Bytes 15 through 8 are non-zero
       capacity = 16;
       }
-    else if( *(uint32_t*)(pvalue + 4) )
+    else if( min_prec > 32 )
       {
       // Bytes 7 through 4 are non-zero
       capacity = 8;
       }
-    else if( *(uint16_t*)(pvalue + 2) )
+    else if( min_prec > 16 )
       {
       // Bytes 3 and 2
       capacity = 4;
       }
-    else if( pvalue[1] )
+    else if( min_prec > 8 )
       {
       // Byte 1 is non-zero
       capacity = 2;
@@ -3940,11 +3937,13 @@ psa_FldLiteralN(struct cbl_field_t *fiel
 
     if( capacity < 16 && (field->attr & signable_e) )
       {
-      if( value < 0 && (((pvalue[capacity-1] & 0x80) == 0 )))
+      FIXED_WIDE_INT(128) mask
+        = wi::set_bit_in_zero<FIXED_WIDE_INT(128)>(capacity * 8 - 1);
+      if( wi::neg_p (value) && (value & mask) == 0 )
         {
         capacity *= 2;
         }
-      else if( value >= 0 && (((pvalue[capacity-1] & 0x80) == 0x80 )))
+      else if( !wi::neg_p (value) && (value & mask) != 0 )
         {
         capacity *= 2;
         }
@@ -3964,86 +3963,15 @@ psa_FldLiteralN(struct cbl_field_t *fiel
 
   tree var_type;
 
-  if( field->data.capacity == 16 )
-    {
-    /*  GCC-13 has no provision for an int128 constructor.  So, we use a
-        union for our necessary __int128.
-
-        typedef union cblc_int128_t
-            {
-            unsigned char array16[16];
-            __uint128     uval128;
-            __int128      sval128;
-            } cblc_int128_t;
-
-      We build a constructor for the array16[], and then we use that
-      constructor in the constructor for the union.
-      */
-
-    // Build the constructor for array16
-    tree array16_type                   = build_array_type_nelts(UCHAR, 16);
-    tree array_16_constructor           = make_node(CONSTRUCTOR);
-    TREE_TYPE(array_16_constructor)     = array16_type;
-    TREE_STATIC(array_16_constructor)   = 1;
-    TREE_CONSTANT(array_16_constructor) = 1;
-
-    for(int i=0; i<16; i++)
-      {
-      CONSTRUCTOR_APPEND_ELT( CONSTRUCTOR_ELTS(array_16_constructor),
-                              build_int_cst_type(INT, i),
-                              build_int_cst_type(UCHAR,
-                                                 ((unsigned char 
*)&value)[i]));
-      }
-
-    // The array16 constructor is ready to be used
-
-    // So, we need a constructor for the union:
-    // Now we create the union:
-    var_type = cblc_int128_type_node;
-
-    tree union_constructor            = make_node(CONSTRUCTOR);
-    TREE_TYPE(union_constructor)      = var_type;
-    TREE_STATIC(union_constructor)    = 1;
-    TREE_CONSTANT(union_constructor)  = 1;
-
-    // point next_field to the first field of the union, and
-    // set the value to be the table constructor
-    tree next_field = TYPE_FIELDS(var_type);
-    CONSTRUCTOR_APPEND_ELT( CONSTRUCTOR_ELTS(union_constructor),
-                            next_field,
-                            array_16_constructor );
-
-    tree new_var_decl = gg_define_variable( var_type,
-                                            base_name,
-                                            vs_static);
-    DECL_INITIAL(new_var_decl) = union_constructor;
-
-    field->data_decl_node = member(new_var_decl, "sval128");
-    TREE_READONLY(field->data_decl_node) = 1;
-    TREE_CONSTANT(field->data_decl_node) = 1;
-
-    // Convert the compile-time data.value to a run-time variable decl node:
-    sprintf(id_string, ".%ld", ++our_index);
-    strcpy(base_name, field->name);
-    strcat(base_name, id_string);
-    field->literal_decl_node = gg_define_variable(DOUBLE, id_string, 
vs_static);
-    TREE_READONLY(field->literal_decl_node) = 1;
-    TREE_CONSTANT(field->literal_decl_node) = 1;
-    tree initer = fold_convert (DOUBLE, field->data.value_of());
-    DECL_INITIAL(field->literal_decl_node) = initer;
-
-    }
-  else
-    {
-    // The value is 1, 2, 4, or 8 bytes, so an ordinary constructor can be 
used.
-    var_type = tree_type_from_size( field->data.capacity,
-                                    field->attr & signable_e);
-    tree new_var_decl = gg_define_variable( var_type,
-                                            base_name,
-                                            vs_static);
-    DECL_INITIAL(new_var_decl) = build_int_cst_type(var_type, value);
-    field->data_decl_node = new_var_decl;
-    }
+  // The value is 1, 2, 4, 8 or 16 bytes, so an ordinary constructor can be
+  // used.
+  var_type = tree_type_from_size( field->data.capacity,
+                                  field->attr & signable_e);
+  tree new_var_decl = gg_define_variable( var_type,
+                                          base_name,
+                                          vs_static);
+  DECL_INITIAL(new_var_decl) = wide_int_to_tree(var_type, value);
+  field->data_decl_node = new_var_decl;
   }
 
 static void
@@ -13739,8 +13667,6 @@ mh_identical(cbl_refer_t &destref,
   return moved;
   }
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpedantic"
 static bool
 mh_source_is_literalN(cbl_refer_t &destref,
                       cbl_refer_t &sourceref,
@@ -14013,7 +13939,6 @@ mh_source_is_literalN(cbl_refer_t &destr
     }
   return moved;
   }
-#pragma GCC diagnostic pop
 
 static
 tree float_type_of(int n)
@@ -15245,8 +15170,6 @@ real_powi10 (uint32_t x)
   return pow10;
 }
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpedantic"
 char *
 binary_initial_from_float128(cbl_field_t *field, int rdigits,
                              REAL_VALUE_TYPE value)
@@ -15322,7 +15245,6 @@ binary_initial_from_float128(cbl_field_t
 
   return retval;
   }
-#pragma GCC diagnostic pop
 
 
 static void
--- gcc/cobol/genutil.cc.jj     2025-03-25 21:14:52.450330315 +0100
+++ gcc/cobol/genutil.cc        2025-03-25 21:19:08.743833202 +0100
@@ -1419,9 +1419,6 @@ get_data_address( cbl_field_t *field,
     }
   }
 
-// Ignore pedantic because we know 128-bit computation is not ISO C++14. 
-#pragma GCC diagnostic ignored "-Wpedantic"
-
 FIXED_WIDE_INT(128)
 get_power_of_ten(int n)
   {


        Jakub

Reply via email to