Bug 100899 - JSC: C++ llint 64-bit backend needs to zero extend results of int32 operations
Summary: JSC: C++ llint 64-bit backend needs to zero extend results of int32 operations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks: 97586
  Show dependency tree / graph
 
Reported: 2012-10-31 15:53 PDT by Mark Lam
Modified: 2012-11-01 00:32 PDT (History)
4 users (show)

See Also:

Attachments
Fix. (3.06 KB, patch)
2012-10-31 15:58 PDT, Mark Lam 		fpizlo: review-
Details | Formatted Diff | Diff
The real fix. (11.02 KB, patch)
2012-10-31 23:40 PDT, Mark Lam 		fpizlo: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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 ===