Bug 100899

Summary: JSC: C++ llint 64-bit backend needs to zero extend results of int32 operations
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, laszlo.gombos, ossy, zherczeg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 97586    
Attachments:
Description Flags
Fix.
fpizlo: review-
The real fix. fpizlo: review+

Description Mark Lam 2012-10-31 15:53:14 PDT
The BaseIndex address calculation should use a different type/sized index depending on the type of the instruction that is using it.  This is an x86_64-ism that wasn't previously captured.
Comment 1 Mark Lam 2012-10-31 15:58:27 PDT
Created attachment 171736 [details]
Fix.
Comment 2 Filip Pizlo 2012-10-31 16:17:43 PDT
Comment on attachment 171736 [details]
Fix.

Mark and I found a bug while looking at this.
Comment 3 Mark Lam 2012-10-31 20:42:47 PDT
Also, the index register used in BaseIndex addressing is expected to be of size intptr_t.
Comment 4 Mark Lam 2012-10-31 23:40:15 PDT
Created attachment 171780 [details]
The real fix.
Comment 5 Filip Pizlo 2012-10-31 23:51:38 PDT
Comment on attachment 171780 [details]
The real fix.

Woohoo!
Comment 6 Mark Lam 2012-11-01 00:32:11 PDT
Landed in r133131: <http://trac.webkit.org/changeset/133131>.

The svn commit message for it was erroneous.  It should have said:
=== BEGIN ===
C++ llint 64-bit backend needs to zero extend results of int32 operations.
https://bugs.webkit.org/show_bug.cgi?id=100899.

Reviewed by Filip Pizlo.

llint asm instructions ending in "i" for a 64-bit machine expects the
high 32-bit of registers to be zero'ed out when a 32-bit instruction
writes into a register. Fixed the C++ llint to honor this.

Fixed the index register used in BaseIndex addressing to be of size
intptr_t as expected.

Updated CLoopRegister to handle different endiannesss configurations.

* llint/LowLevelInterpreter.cpp:
  (JSC::CLoopRegister::clearHighWord):
  - new method to clear the high 32-bit of a 64-bit register.
     It's a no-op for the 32-bit build. 
  (CLoopRegister):
  - CLoopRegister now takes care of packing and byte endianness order.
  (JSC::CLoop::execute): - Added an assert.
* offlineasm/cloop.rb:
   - Add calls to clearHighWord() wherever needed.
=== END ===