diff mbox

[Fortran] Fix integer kind returned by storage_size (was: Re: incorrect integer kind returned from call to storage_size() with gcc 4.9.0)

Message ID 53D29EDA.4040800@net-b.de
State New
Headers show

Commit Message

Tobias Burnus July 25, 2014, 6:15 p.m. UTC 
Hi,

N.M. Maclaren wrote:
> On Jul 25 2014, Rezny, Mike wrote:
>>
>> I am seeing the following problems in using the Fortran intrinsic 
>> function, storage_size(), in gfortran version 4.9.0. 1: A calls to 
>> this function is returning a 64-bit integer instead of a default 
>> 32-bit integer 2: the routine is not honouring the second parameter 
>> to the call which specifies the kind of the returned value. In all 
>> valid cases, the returned value is a 64-bit integer.
>
> The key property is the KIND of the result - if THAT is not KIND(0),
> then there is a bug.

I can confirm the problem. It only occurs when the compiler can simplify 
the intrinsic function call at compile time (what it usually can for 
storage_size). Thus, if one passes a polymorphic argument, the KIND 
value is properly handled. By the way, the used kind is "c_ptrdiff_t" 
from the intrinsic module ISO_C_Binding.

In case a work around is needed, use "int(storage_size(...), kind=...)". 
However, in case you pass the value on to some run-time library, you 
should consider using "integer(c_ptrdiff_t)" [TS 29113] or 
"integer(c_size_t)" [F2003] instead. In general, it makes sense to 
handle as large storage_sizes as the system permits. (ptrdiff_t is 
signed, which matches what Fortran uses as only signed integers are 
supported; size_t is unsigned but of the same storage size; as 
"c_size_t" is already in Fortran 2003 many more compilers support it 
than "c_ptrdiff_t".)


The problem is fixed by the attached patch. I will commit it as obvious 
(to the trunk, i.e. GCC 5 alias GCC 4.10 only) once building and 
regtesting has finished.

Thanks for reporting the bug – and sorry for the inconvenience.

Tobias

Comments

Tobias Burnus July 25, 2014, 6:25 p.m. UTC | #1 
Tobias Burnus wrote:
> The problem is fixed by the attached patch. I will commit it as 
> obvious (to the trunk, i.e. GCC 5 alias GCC 4.10 only) once building 
> and regtesting has finished.

As I only saw later in Steve's email, it is a regression. Thus, I will 
also apply it to the GCC 4.9 branch (will be in GCC 4.9.2; Linux distros 
likely pick it up earlier).

The regression was caused by my commit r197159 for PRs 56650 and 36437, 
which added compile-time simplification for storage_size, c_sizeof and 
sizeof. (The latter are not affected as they are supposed to return a 
value of kind c_size_t and take no kind parameters.)

Tobias
diff mbox

Patch

2014-07-25  Tobias Burnus  <burnus@net-b.de>

	* simplify.c (gfc_simplify_storage_size): Use proper
	integer kind for the returned value.

2014-07-25  Tobias Burnus  <burnus@net-b.de>

	* gfortran.dg/storage_size_5.f90: New.

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 60d8593..d4a67ad 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -5841,11 +5841,9 @@  gfc_simplify_storage_size (gfc_expr *x,
   if (k == -1)
     return &gfc_bad_expr;
 
-  result = gfc_get_constant_expr (BT_INTEGER, gfc_index_integer_kind,
-				  &x->where);
+  result = gfc_get_constant_expr (BT_INTEGER, k, &x->where);
 
   mpz_set_si (result->value.integer, gfc_element_size (x));
-
   mpz_mul_ui (result->value.integer, result->value.integer, BITS_PER_UNIT);
 
   return range_check (result, "STORAGE_SIZE");
diff --git a/gcc/testsuite/gfortran.dg/storage_size_5.f90 b/gcc/testsuite/gfortran.dg/storage_size_5.f90
new file mode 100644
index 0000000..ae0f126
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/storage_size_5.f90
@@ -0,0 +1,44 @@ 
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+subroutine test()
+  implicit none
+  integer :: i0, i1, i2, i3, i4
+  i0 = kind(STORAGE_SIZE(5))
+  i1 = kind(STORAGE_SIZE(5, kind=1))
+  i2 = kind(STORAGE_SIZE(5, kind=2))
+  i3 = kind(STORAGE_SIZE(5, kind=4))
+  i4 = kind(STORAGE_SIZE(5, kind=8))
+end subroutine test
+
+subroutine test2(x)
+  implicit none
+  class(*) :: x
+  integer :: j0, j1, j2, j3, j4
+  integer(1) :: k1
+  integer(2) :: k2
+  j0 = kind(STORAGE_SIZE(x))
+  j1 = kind(STORAGE_SIZE(x, kind=1))
+  j2 = kind(STORAGE_SIZE(x, kind=2))
+  j3 = kind(STORAGE_SIZE(x, kind=4))
+  j4 = kind(STORAGE_SIZE(x, kind=8))
+
+  k1 = STORAGE_SIZE(x, kind=1)
+  k2 = STORAGE_SIZE(x, kind=2)
+end subroutine test2
+
+! { dg-final { scan-tree-dump-times "i0 = 4;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "i1 = 1;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "i2 = 2;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "i3 = 4;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "i4 = 8;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "j0 = 4;" 1 "original" } }
+
+! { dg-final { scan-tree-dump-times "j1 = 1;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "j2 = 2;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "j3 = 4;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "j4 = 8;" 1 "original" } }
+
+! { dg-final { scan-tree-dump-times "k1 = \\(integer\\(kind=1\\)\\)" 1 "original" } }
+! { dg-final { scan-tree-dump-times "k2 = \\(integer\\(kind=2\\)\\)" 1 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }