Bug 133955

Summary: css jit should not save callee saved registers
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: CSSAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bunhere, cdumez, commit-queue, gyuyoung.kim, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch ggaren: review+

Description Alex Christensen 2014-06-16 15:01:20 PDT
We do a lot of pushing and popping that we don't need to, especially on armv7 and x86_64.  This could minimize it.
Comment 1 Alex Christensen 2014-06-16 15:05:41 PDT
Created attachment 233185 [details]
Patch
Comment 2 Alex Christensen 2014-06-17 12:41:29 PDT
Comment on attachment 233185 [details]
Patch

We need a way to specify whether we want to save the values in the argument registers.
Comment 3 Alex Christensen 2014-06-27 10:43:40 PDT
Created attachment 233994 [details]
Patch
Comment 4 Alex Christensen 2014-06-27 11:41:55 PDT
Created attachment 234001 [details]
Patch
Comment 5 Geoffrey Garen 2014-06-27 12:56:36 PDT
Comment on attachment 234001 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234001&action=review

r=me

> Source/WebCore/cssjit/SelectorCompiler.cpp:1442
>      // FIXME: We should look into doing something smart in MacroAssembler instead.
>      Assembler::Address flagAddress(childStyle, RenderStyle::noninheritedFlagsMemoryOffset() + RenderStyle::NonInheritedFlags::flagsMemoryOffset());
>  #if CPU(ARM_THUMB2)
> -    m_assembler.or32(Assembler::TrustedImm32(newFlag & 0xffffffff), flagAddress);
> -    Assembler::Address flagHighBits = flagAddress.withOffset(4);
> -    m_assembler.or32(Assembler::TrustedImm32(newFlag >> 32), flagHighBits);
> +    int32_t flagLowBits = newFlag & 0xffffffff;
> +    int32_t flagHighBits = newFlag >> 32;
> +    if (flagLowBits)
> +        m_assembler.or32(Assembler::TrustedImm32(flagLowBits), flagAddress);
> +    if (flagHighBits) {
> +        Assembler::Address flagHighAddress = flagAddress.withOffset(4);
> +        m_assembler.or32(Assembler::TrustedImm32(flagHighBits), flagHighAddress);
> +    }

A good optimization, but probably better as a separate patch, since the topic is not related to register saving.
Comment 6 Alex Christensen 2014-06-27 13:39:01 PDT
http://trac.webkit.org/changeset/170548