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+

Mark Lam
Reported 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.
Attachments
Fix. (3.06 KB, patch)
2012-10-31 15:58 PDT, Mark Lam 		fpizlo: review-
DetailsFormatted DiffDiff
The real fix. (11.02 KB, patch)
2012-10-31 23:40 PDT, Mark Lam 		fpizlo: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2012-10-31 15:58:27 PDT
Created attachment 171736 [details] Fix.
Filip Pizlo
Comment 2 2012-10-31 16:17:43 PDT
Comment on attachment 171736 [details] Fix. Mark and I found a bug while looking at this.
Mark Lam
Comment 3 2012-10-31 20:42:47 PDT
Also, the index register used in BaseIndex addressing is expected to be of size intptr_t.
Mark Lam
Comment 4 2012-10-31 23:40:15 PDT
Created attachment 171780 [details] The real fix.
Filip Pizlo
Comment 5 2012-10-31 23:51:38 PDT
Comment on attachment 171780 [details] The real fix. Woohoo!
Mark Lam
Comment 6 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 ===
Note You need to log in before you can comment on or make changes to this bug.