This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, fortran] Implement constant-folding for TRANSFER intrinsic.



Hi Brooks,


this looks good for the most part. I have a few remarks, though.

Brooks Moses wrote:
+  /* Calculate the size of the source.  */
+  if (source->expr_type == EXPR_ARRAY
+      && gfc_array_size (source, &tmp) == FAILURE)
+    gfc_internal_error ("Failure getting length of a constant array.");

I think we're standardizing on gcc_assert even in the frontend.


+ if (size)
^^^^
Above you checked size != NULL. I'd prefer if you omitted that in both cases.


+	result_length = (size_t)mpz_get_ui (size->value.integer);
+      else
+	{
+	  result_length = source_size / result_elt_size;
+	  if (result_length * result_elt_size < source_size)
+	    result_length += 1;
+	}

result_length = (source_size - 1) / result_elt_size + 1;


Don't know if that's harder to understand.

+  /* Allocate the buffer to store the binary version of the source.  */
+  buffer_size = source_size > result_size ? source_size : result_size;

buffer_size = MAX (source_size, result_size);


+ buffer = gfc_getmem (buffer_size);

Maybe use alloca?


+/* ---------------------------------------------------------------  */
+/* Calculate the size of an expression.  */
+
+static size_t
+size_array (gfc_expr *e)
+{
+  mpz_t array_size;
+  size_t elt_size = gfc_target_expr_size (e->value.constructor->expr);
+
+  gfc_array_size (e, &array_size);
+  return (size_t)mpz_get_ui (array_size) * elt_size;
+}
+
+static size_t
+size_integer (int kind)
+{
+  return GET_MODE_SIZE (TYPE_MODE (gfc_get_int_type (kind)));;
+}
+
+
+static size_t
+size_float (int kind)
+{
+  return GET_MODE_SIZE (TYPE_MODE (gfc_get_real_type (kind)));;
+}
+
+
+static size_t
+size_complex (int kind)
+{
+  return kind * size_float (kind);
            ^^^^
Should be 2.  You also lack testcases involving COMPLEX.

+    case BT_DERIVED:
+      ctr = e->value.constructor;
+      for (;ctr; ctr = ctr->next)
+	{
+	  gcc_assert (ctr->expr != NULL);
+	  expr_size += gfc_target_expr_size (ctr->expr);
+	}
+      return expr_size;

Is that so? What about alignment, say, of INTEGER*1 components? This seems to call for the middle-end's size_for_type().


+static int
+encode_complex (int kind, mpfr_t real, mpfr_t imaginary, unsigned char *buffer,
+		size_t buffer_size)
+{
+  int size;
+  size = encode_float (kind, real, &buffer[0], buffer_size);
+  size += encode_float (kind, imaginary, &buffer[size], buffer_size - size);
+  return size;
+


+static int
+encode_character (int length, char *string, unsigned char *buffer,
+		  size_t buffer_size)
+{
+  gcc_assert (buffer_size >= size_character (length));
+  memcpy (buffer, string, length);
+  return length;
+}

What about targets with different character sets?


+static int
+encode_derived (gfc_expr *source, unsigned char *buffer, size_t buffer_size)
+{
+  gfc_constructor *ctr;
+  int ptr = 0;
+
+  ctr = source->value.constructor;
+  for (;ctr; ctr = ctr->next)
+    {
+      gcc_assert (ctr->expr != NULL);
+      ptr += gfc_target_encode_expr (ctr->expr, &buffer[ptr],
+				     buffer_size - ptr);
+    }
+  return ptr;
+}

Again, alignment may interfere with this logic.


+  /* Calculate array size from its shape and rank.  */
+  if (result->rank == 0 || result->shape == NULL)
+    gfc_error ("failure to obtain array size at %L", &result->where);

This should probably be caught in gfc_check_transfer, and at most gcc_asserted here.


Cheers,
- Tobi


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]