Opened 6 years ago

Closed 6 years ago

#2223 closed merge (fixed)

Int64.toInteger

Reported by: gnezdo Owned by: igloo
Priority: normal Milestone: 6.8.3
Component: Runtime System Version: 6.8.2
Keywords: Cc: wolfgang.thaller@…
Operating System: Linux Architecture: x86
Type of failure: Difficulty: Unknown
Test Case: arith011 Blocked By:
Blocking: Related Tickets:

Description

GHCi, version 6.8.2: http://www.haskell.org/ghc/  :? for help
Loading package base ... linking ... done.
Prelude> 0 == (-1 `Data.Bits.shift` 32 :: Data.Int.Int64)
False
Prelude> 0 == toInteger (-1 `Data.Bits.shift` 32 :: Data.Int.Int64)
True
-- ^^^ BUG

The problem appears to be in the recently changed
rts/PrimOps.cmm::int64ToIntegerzh_fast:

   if ( hi != 0 && hi != 0xFFFFFFFF )  { 
      words_needed = 2;
   } else { 
       // minimum is one word
       words_needed = 1;
   }

So, 0xFFFFFFFF00000000 becomes 0.

Attachments (1)

ghc-6.8.2-Int64-toInteger.patch (512 bytes) - added by gnezdo 6 years ago.
Proposed patch. Courtesy of Mike Gunter.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by gnezdo

Proposed patch. Courtesy of Mike Gunter.

comment:1 Changed 6 years ago by gnezdo

  • Cc wolfgang.thaller@… added

A bit of source browsing indicates this could be the patch that introduced the code in question.

Fri Nov 24 01:41:29 PST 2006  wolfgang.thaller@gmx.net
  * Support I64->I32 casts in the NCG, and use them for I64->Integer conversions
  
  We can avoid using any other long long operations in PrimOps.cmm.
  One more step towards compiling the RTS using the NCG.

Wolfgang, could you comment on the correctness of the proposed patch?

comment:2 Changed 6 years ago by wolfgang

Yes, my code was wrong.

I think your code still leaves out most corner cases:
Consider 0xFFFFFFFF00000001, or 0x0000000080000000.

If the high word is 0 of -1 , and the sign bit of the low word matches that, THEN one word is enough.
So I would recommend the following (untested):

	   if ( (hi == 0 && %ge( lo, 0 )) || (hi == 0xFFFFFFFF && %lt(lo, 0) )  {  
	       words_needed = 1; 
	       // minimum is one word 
	   } else {  
	       words_needed = 2; 
	   } 

comment:3 Changed 6 years ago by igloo

  • Difficulty set to Unknown
  • Owner set to igloo
  • Test Case set to arith011

comment:4 Changed 6 years ago by igloo

  • Milestone set to 6.8.3

comment:5 Changed 6 years ago by igloo

  • Type changed from bug to merge

I've applied the original (Mike Gunter's) patch; it is sufficient as far as I can see.

comment:6 Changed 6 years ago by igloo

  • Resolution set to fixed
  • Status changed from new to closed

Merged

Thu Apr 24 14:15:26 BST 2008  Ian Lynagh <igloo@earth.li>
  * Fix int64ToInteger 0xFFFFFFFF00000000 on 32bit machine; trac #2223
  Patch from Mike Gunter.
Note: See TracTickets for help on using tickets.