Attachment 214050 Details for Bug 122688 – the patch

[patch] the patch

blah.patch (text/plain), 3.86 KB, created by Filip Pizlo on 2013-10-11 21:45:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2013-10-11 21:45:16 PDT

Size: 3.86 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 157330)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,38 @@
>+2013-10-11  Filip Pizlo  <fpizlo@apple.com>
>+
>+        FTL OSR exit should perform zero extension on values smaller than 64-bit
>+        https://bugs.webkit.org/show_bug.cgi?id=122688
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        In the DFG we usually make the simplistic assumption that a 32-bit value in a 64-bit
>+        register will have zeros on the high bits.  In the few cases where the high bits are
>+        non-zero, the DFG sort of tells us this explicitly.
>+
>+        But when working with llvm.webkit.stackmap, it doesn't work that way.  Consider we might
>+        emit LLVM IR like:
>+
>+            %2 = trunc i64 %1 to i32
>+            stuff %2
>+            call @llvm.webkit.stackmap(...., %2)
>+
>+        LLVM may never actually emit a truncation instruction of any kind.  And that's great - in
>+        many cases it won't be needed, like if 'stuff %2' is a 32-bit op that ignores the high
>+        bits anyway.  Hence LLVM may tell us that %2 is in the register that still had the value
>+        from before truncation, and that register may have garbage in the high bits.
>+
>+        This means that on our end, if we want a 32-bit value and we want that value to be
>+        zero-extended, we should zero-extend it ourselves.  This is pretty easy and should be
>+        cheap, so we should just do it and not make it a requirement that LLVM does it on its
>+        end.
>+        
>+        This makes all tests pass with JSC_ftlOSRExitUsesStackmap=true.
>+
>+        * ftl/FTLOSRExitCompiler.cpp:
>+        (JSC::FTL::compileStubWithOSRExitStackmap):
>+        * ftl/FTLValueFormat.cpp:
>+        (JSC::FTL::reboxAccordingToFormat):
>+
> 2013-10-11  Darin Adler  <darin@apple.com>
> 
>         Change most call sites to call ICU directly instead of through WTF::Unicode
>Index: Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp	(revision 157330)
>+++ Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp	(working copy)
>@@ -171,10 +171,11 @@ static void compileStubWithOSRExitStackm
>     exit.m_code = FINALIZE_CODE_IF(
>         shouldShowDisassembly(),
>         patchBuffer,
>-        ("FTL OSR exit #%u (bc#%u, %s) from %s, with operands = %s",
>+        ("FTL OSR exit #%u (bc#%u, %s) from %s, with operands = %s, and record = %s",
>             exitID, exit.m_codeOrigin.bytecodeIndex,
>             exitKindToString(exit.m_kind), toCString(*codeBlock).data(),
>-            toCString(ignoringContext<DumpContext>(exit.m_values)).data()));
>+            toCString(ignoringContext<DumpContext>(exit.m_values)).data(),
>+            toCString(*record).data()));
> }
> 
> static void compileStubWithoutOSRExitStackmap(
>Index: Source/JavaScriptCore/ftl/FTLValueFormat.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLValueFormat.cpp	(revision 157330)
>+++ Source/JavaScriptCore/ftl/FTLValueFormat.cpp	(working copy)
>@@ -37,11 +37,13 @@ void reboxAccordingToFormat(
> {
>     switch (format) {
>     case ValueFormatInt32: {
>+        jit.zeroExtend32ToPtr(value, value);
>         jit.or64(GPRInfo::tagTypeNumberRegister, value);
>         break;
>     }
>     
>     case ValueFormatUInt32: {
>+        jit.zeroExtend32ToPtr(value, value);
>         jit.moveDoubleTo64(FPRInfo::fpRegT0, scratch2);
>         jit.boxInt52(value, value, scratch1, FPRInfo::fpRegT0);
>         jit.move64ToDouble(scratch2, FPRInfo::fpRegT0);
>@@ -64,6 +66,7 @@ void reboxAccordingToFormat(
>     }
>     
>     case ValueFormatBoolean: {
>+        jit.zeroExtend32ToPtr(value, value);
>         jit.or32(MacroAssembler::TrustedImm32(ValueFalse), value);
>         break;
>     }

Flags:

barraclough: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 122688: 214049 | 214050