[patch,libfortran] PR31964 ishftc fails with certain thrid argument

[patch,libfortran] PR31964 ishftc fails with certain thrid argument

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

Hi,

Fortran defines bit shifting up to and including bit_size of a value.

It turns out that shifting is undefined for >= bit_size bits.  In the test case 
reported, we are trying to shift a 32 bit word, 32 times.  The ishftc function 
tried to do this, but no can do. (Thank you Andrew for pointing that out and I 
confirmed it looking in K&R)

To fix this, the patch special cases the mask for this situation.  I took the 
liberty to do a few other minor tweaks so that the result looks logical to me. 
  In particular, not using the original incoming word as operand for the right 
shift.  I also arranged the shifting left to right in the result statement.

Updated test case is attached.

Regression tested on X86-64.

OK for trunk and I assume 4.2 when open, and 4.1 after that?

Regards,

Jerry

2007-05-17  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/31964
	* intrinsics/ishftc.c (ishftc4, ishftc8, ishftc16): Fix mask to handle
	shift of bit-size number of bits.



[-- Attachment #2: pr31964.diff --]
[-- Type: text/x-patch, Size: 3802 bytes --]

Index: ishftc.c
===================================================================
*** ishftc.c	(revision 124756)
--- ishftc.c	(working copy)
*************** export_proto(ishftc4);
*** 36,43 ****
  GFC_INTEGER_4
  ishftc4 (GFC_INTEGER_4 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_INTEGER_4 mask;
!   GFC_UINTEGER_4 bits;
  
    if (shift < 0)
      shift = shift + size;
--- 36,42 ----
  GFC_INTEGER_4
  ishftc4 (GFC_INTEGER_4 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_UINTEGER_4 mask, bits;
  
    if (shift < 0)
      shift = shift + size;
*************** ishftc4 (GFC_INTEGER_4 i, GFC_INTEGER_4 
*** 45,53 ****
    if (shift == 0 || shift == size)
      return i;
  
!   mask = (~(GFC_INTEGER_4)0) << size;
!   bits = i & ~mask;
!   return (i & mask) | (bits >> (size - shift)) | ((i << shift) & ~mask);
  }
  
  extern GFC_INTEGER_8 ishftc8 (GFC_INTEGER_8, GFC_INTEGER_4, GFC_INTEGER_4);
--- 44,57 ----
    if (shift == 0 || shift == size)
      return i;
  
!   /* In C, the result of the shift operator is undefined if the right operand
!      is greater than or equal to the number of bits in the left operand. So we
!      have to special case it for fortran.  */
!   mask = ~((size == 32) ? 0 : (~0 << size));
! 
!   bits = i & mask;
!   
!   return (i & ~mask) | ((bits << shift) & mask) | (bits >> (size - shift));
  }
  
  extern GFC_INTEGER_8 ishftc8 (GFC_INTEGER_8, GFC_INTEGER_4, GFC_INTEGER_4);
*************** export_proto(ishftc8);
*** 56,63 ****
  GFC_INTEGER_8
  ishftc8 (GFC_INTEGER_8 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_INTEGER_8 mask;
!   GFC_UINTEGER_8 bits;
  
    if (shift < 0)
      shift = shift + size;
--- 60,66 ----
  GFC_INTEGER_8
  ishftc8 (GFC_INTEGER_8 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_UINTEGER_8 mask, bits;
  
    if (shift < 0)
      shift = shift + size;
*************** ishftc8 (GFC_INTEGER_8 i, GFC_INTEGER_4 
*** 65,73 ****
    if (shift == 0 || shift == size)
      return i;
  
!   mask = (~(GFC_INTEGER_8)0) << size;
!   bits = i & ~mask;
!   return (i & mask) | (bits >> (size - shift)) | ((i << shift) & ~mask);
  }
  
  #ifdef HAVE_GFC_INTEGER_16
--- 68,81 ----
    if (shift == 0 || shift == size)
      return i;
  
!   /* In C, the result of the shift operator is undefined if the right operand
!      is greater than or equal to the number of bits in the left operand. So we
!      have to special case it for fortran.  */
!   mask = ~((size == 64) ? 0 : (~0 << size));
! 
!   bits = i & mask;
!   
!   return (i & ~mask) | ((bits << shift) & mask) | (bits >> (size - shift));
  }
  
  #ifdef HAVE_GFC_INTEGER_16
*************** export_proto(ishftc16);
*** 77,84 ****
  GFC_INTEGER_16
  ishftc16 (GFC_INTEGER_16 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_INTEGER_16 mask;
!   GFC_UINTEGER_16 bits;
  
    if (shift < 0)
      shift = shift + size;
--- 85,91 ----
  GFC_INTEGER_16
  ishftc16 (GFC_INTEGER_16 i, GFC_INTEGER_4 shift, GFC_INTEGER_4 size)
  {
!   GFC_UINTEGER_16 mask, bits;
  
    if (shift < 0)
      shift = shift + size;
*************** ishftc16 (GFC_INTEGER_16 i, GFC_INTEGER_
*** 86,93 ****
    if (shift == 0 || shift == size)
      return i;
  
!   mask = (~(GFC_INTEGER_16)0) << size;
!   bits = i & ~mask;
!   return (i & mask) | (bits >> (size - shift)) | ((i << shift) & ~mask);
  }
  #endif
--- 93,105 ----
    if (shift == 0 || shift == size)
      return i;
  
!   /* In C, the result of the shift operator is undefined if the right operand
!      is greater than or equal to the number of bits in the left operand. So we
!      have to special case it for fortran.  */
!   mask = ~((size == 128) ? 0 : (~0 << size));
! 
!   bits = i & mask;
!   
!   return (i & ~mask) | ((bits << shift) & mask) | (bits >> (size - shift));
  }
  #endif

[-- Attachment #3: intrinsic_bitops.f90 --]
[-- Type: text/x-fortran, Size: 993 bytes --]

! Program to test intrinsic bitops
program intrinsic_bitops
   implicit none
   integer(kind=4) :: i, j, k, o, t
   integer(kind=8) :: a, b, c

   o = 0
   i = 2
   j = 3
   k = 12
   a = 5
   
   if (.not. btest (i, o+1)) call abort
   if (btest (i, o+2)) call abort
   if (iand (i, j) .ne. 2) call abort
   if (ibclr (j, o+1) .ne. 1) call abort
   if (ibclr (j, o+2) .ne. 3) call abort
   if (ibits (k, o+1, o+2) .ne. 2) call abort
   if (ibset (j, o+1) .ne. 3) call abort
   if (ibset (j, o+2) .ne. 7) call abort
   if (ieor (i, j) .ne. 1) call abort
   if (ior (i, j) .ne. 3) call abort
   if (ishft (k, o+2) .ne. 48) call abort
   if (ishft (k, o-3) .ne. 1) call abort
   if (ishft (k, o) .ne. 12) call abort
   if (ishftc (k, o+30) .ne. 3) call abort
   if (ishftc (k, o-30) .ne. 48) call abort
   if (ishftc (k, o+1, o+3) .ne. 9) call abort
   if (not (i) .ne. -3) call abort
   if (ishftc (a, 1, bit_size(a)) .ne. 10) call abort
   if (ishftc (1, 1, 32) .ne. 2) call abort
end program

Re: [patch,libfortran] PR31964 ishftc fails with certain thrid argument

[Toon Moene]

Jerry DeLisle wrote:

> Fortran defines bit shifting up to and including bit_size of a value.
> 
> It turns out that shifting is undefined for >= bit_size bits.

Ugh, I'm sorry this wasn't know (I know it for years, because of 
bringing g77 up to date with the various standards).

Fortran allows shifts up to the width of the item.

C allows shifts up to the width minus one of the time.

Hope this helps,

-- 
Toon Moene - e-mail: toon@moene.indiv.nluug.nl - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
At home: http://moene.indiv.nluug.nl/~toon/
Who's working on GNU Fortran: 
http://gcc.gnu.org/ml/gcc/2007-01/msg00059.html

Re: [patch,libfortran] PR31964 ishftc fails with certain thrid argument

[Jerry DeLisle]

Toon Moene wrote:
> Jerry DeLisle wrote:
> 
>> Fortran defines bit shifting up to and including bit_size of a value.
>>
>> It turns out that shifting is undefined for >= bit_size bits.
> 
> Ugh, I'm sorry this wasn't know (I know it for years, because of 
> bringing g77 up to date with the various standards).
> 
> Fortran allows shifts up to the width of the item.
> 
> C allows shifts up to the width minus one of the time.
> 
> Hope this helps,
> 
Hi Toon,

Already sorted this out and a patch is submitted.  If you could review and OK 
the patch, that would be helpful.

Jerry

Re: [patch,libfortran] PR31964 ishftc fails with certain thrid argument

[Tobias Schlüter]

Jerry DeLisle wrote:
> Regression tested on X86-64.
> 
> OK for trunk and I assume 4.2 when open, and 4.1 after that?

This made my head spin, but it's ok.  If you wouldn't negate mask, but 
instead negated in the other expressions, you wouldn't reach the point 
of negating three times, but I don't want to think this through to where 
I'm sure enough to demand that change :-)

Cheers,
- Tobi