Hi Andre,
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index 66a3635404a..a214b8bc1b3 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -711,17 +711,9 @@ gfc_arith_uminus (gfc_expr *op1, gfc_expr **resultp)
case BT_UNSIGNED:
{
if (pedantic)
- return ARITH_UNSIGNED_NEGATIVE;
+ return check_result (ARITH_UNSIGNED_NEGATIVE, op1, result, resultp);
What is the need for this check? ARITH_UNSIGNED_NEGATIVE is, when I read the
code correctly, never triggering an error here. What do I not see?
check_result is a bit of a misnomer, it actually assigns the result
to resultp, or frees the operand if a hard arithmetic error is
encountered. I had the same thought you had, but learned abou this
by encountering an ICE :-)
- arith neg_rc;
mpz_neg (result->value.integer, op1->value.integer);
- neg_rc = gfc_range_check (result);
<SNIP>
diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index cfafdb7974f..7c630dd73f4 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -2804,6 +2804,10 @@ gfc_check_dot_product (gfc_expr *vector_a, gfc_expr
*vector_b) return false;
break;
+ case BT_UNSIGNED:
+ /* Check comes later. */
+ break;
+
default:
gfc_error ("%qs argument of %qs intrinsic at %L must be numeric "
"or LOGICAL", gfc_current_intrinsic_arg[0]->name,
@@ -2811,6 +2815,14 @@ gfc_check_dot_product (gfc_expr *vector_a, gfc_expr
*vector_b) return false;
}
+ if (gfc_invalid_unsigned_ops (vector_a, vector_b))
I haven't read the proposal (shame to me), but why would want not want to
combine a unsigned vector with a signed one? This all depends on the data type
of the result (variable). So why is this needed here? (I know we don't have the
result available here.) It just feels odd to me.
No, we don't. The check does indeed come later, in the
call to gfc_invalid_unsigned_ops case. Hence the comment.
+ {
+ gfc_error ("Argument types of %qs intrinsic at %L must match (%s/%s)",
+ gfc_current_intrinsic, &vector_a->where,
+ gfc_typename(&vector_a->ts), gfc_typename(&vector_b->ts));
+ return false;
+ }
+
if (!rank_check (vector_a, 0, 1))
return false;
@@ -4092,7 +4104,8 @@ gfc_check_matmul (gfc_expr *matrix_a, gfc_expr
*matrix_b) }
if ((matrix_a->ts.type == BT_LOGICAL && gfc_numeric_ts (&matrix_b->ts))
- || (gfc_numeric_ts (&matrix_a->ts) && matrix_b->ts.type == BT_LOGICAL))
+ || (gfc_numeric_ts (&matrix_a->ts) && matrix_b->ts.type == BT_LOGICAL)
+ || gfc_invalid_unsigned_ops (matrix_a, matrix_b))
Same here.
See above.
{
gfc_error ("Argument types of %qs intrinsic at %L must match (%s/%s)",
gfc_current_intrinsic, &matrix_a->where,
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 81c641e2322..cef971894ea 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -224,7 +224,19 @@ gfc_get_int_expr (int kind, locus *where, HOST_WIDE_INT
value) return p;
}
+/* Get a new expression node that is an unsigned constant. */
+gfc_expr *
+gfc_get_unsigned_expr (int kind, locus *where, HOST_WIDE_INT value)
+{
+ gfc_expr *p;
+ p = gfc_get_constant_expr (BT_UNSIGNED, kind,
+ where ? where : &gfc_current_locus);
+ const wide_int w = wi::shwi (value, kind * BITS_PER_UNIT);
+ wi::to_mpz (w, p->value.integer, UNSIGNED);
+
+ return p;
+}
Newline please :-)
OK :-)
/* Get a new expression node that is a logical constant. */
gfc_expr *
<SNIP>
diff --git a/libgfortran/m4/iparm.m4 b/libgfortran/m4/iparm.m4
index b474620424b..0c4c76c2428 100644
--- a/libgfortran/m4/iparm.m4
+++ b/libgfortran/m4/iparm.m4
@@ -4,7 +4,7 @@ dnl This file is part of the GNU Fortran 95 Runtime Library
(libgfortran) dnl Distributed under the GNU GPL with exception. See COPYING
for details. dnl M4 macro file to get type names from filenames
define(get_typename2, `GFC_$1_$2')dnl
-define(get_typename,
`get_typename2(ifelse($1,i,INTEGER,ifelse($1,r,REAL,ifelse($1,l,LOGICAL,ifelse($1,c,COMPLEX,ifelse($1,s,UINTEGER,unknown))))),`$2')')dnl
+define(get_typename,
`get_typename2(ifelse($1,i,INTEGER,ifelse($1,r,REAL,ifelse($1,l,LOGICAL,ifelse($1,c,COMPLEX,ifelse($1,m,UINTEGER,ifelse($1,s,UINTEGER,unknown)))))),`$2')')dnl
Curiosity killed the cat: So type letter 's' and 'm' both signify a unsigned
integer, right? Is there anywhere a notable difference? I meant, keep it simple
is usually wanted and having to type letters with identical meaning is not
simple, right?
Yep. This is a case of "do not meddle in the affairs in m4, for it is
subtle and quick to cause weird errors". I wasn't totally sure that
"s" was used nowhere, so I just stuck it on there as an additional
optin.
define(get_arraytype, `gfc_array_$1$2')dnl define(define_type, `dnl
ifelse(regexp($2,`^[0-9]'),-1,`dnl diff --git a/libgfortran/m4/matmul.m4
b/libgfortran/m4/matmul.m4 index 7fc1f5fa75f..cd804e8be06 100644
--- a/libgfortran/m4/matmul.m4
+++ b/libgfortran/m4/matmul.m4
@@ -28,6 +28,9 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see #include <assert.h>'
include(iparm.m4)dnl
+ifelse(index(rtype_name,`GFC_INTEGER'),`0',dnl
+define(`rtype_name',patsubst(rtype_name,`GFC_INTEGER',`GFC_UINTEGER'))dnl
+define(`rtype',patsubst(rtype,`gfc_array_i',`gfc_array_m')))dnl
Is this correct? Whenever a GFC_INTEGER is seen, it is substituted by
GFC_UINTEGER. How are then INTEGER variants generated? Or are they not needed
anymore? I would have assumed, that when GFC_UINTEGER is the input, we
substitute this with GFC_INTEGER, as was said is the intention and would be
more natural to me, because most of the unsigneds are in the signed domain,
too, but the other way around ...
That was Richard's idea. All we are doing is addition and
multiplication, and you can freely mix signed and unsigned
in C (implementation-defined, I think, with gfortran settled
on two's complement numbers).
The nice thing about two's complement numbers is that addition and
multiplication (for n bit * n bit -> n bit) are the same, so you can
perform any operation either in signed or unsigned. If addition or
multiplication overflow for signed integers, the behavior is undefined
(and the compiler might take advantage of this), so this is best.
If you use unsigned for the operations, the behavior is defined
(modulo 2^n), so you can use unsigned for both.
So, by simply switching to UNSIGNED, we only need a single library
version. But we want to keep the old function name, hence the
gyrations of switching the name to 'i' in the front end and using
unsigned in the library.
`#if defined (HAVE_'rtype_name`)
<SNIP>
* arith.cc (gfc_arith_uminus): Fix warning.
(gfc_arith_minus): Correctly truncate unsigneds.
* check.cc (gfc_check_dot_product): Handle unsigned arguments.
(gfc_check_matmul): Likewise.
* expr.cc (gfc_get_unsigned_expr): New function.
* gfortran.h (gfc_get_unsigned_expr): Add prototype.
* iresolve.cc (gfc_resolve_matmul): If using UNSIGNED, use the
signed integer version.
* gfortran.texi: Document MATMUL and DOT_PRODUCT for unsigned.
* simplify.cc (compute_dot_product): Handle unsigneds.
* arith.cc (gfc_arith_uminus):
(gfc_arith_minus):
* check.cc (gfc_check_dot_product):
(gfc_check_matmul):
* expr.cc (gfc_get_unsigned_expr):
* gfortran.h (gfc_get_unsigned_expr):
* gfortran.texi:
* simplify.cc (compute_dot_product):
Duplicate block in Changelog?
Yep. Will be removed forthwith :-)
Thanks for looking at this!
Best regards
Thomas
