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+

Alex Christensen
Reported 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.
Attachments
Patch (5.79 KB, patch)
2014-06-16 15:05 PDT, Alex Christensen
no flags
Patch (7.18 KB, patch)
2014-06-27 10:43 PDT, Alex Christensen
no flags
Patch (7.19 KB, patch)
2014-06-27 11:41 PDT, Alex Christensen
ggaren: review+
Alex Christensen
Comment 1 2014-06-16 15:05:41 PDT
Alex Christensen
Comment 2 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.
Alex Christensen
Comment 3 2014-06-27 10:43:40 PDT
Alex Christensen
Comment 4 2014-06-27 11:41:55 PDT
Geoffrey Garen
Comment 5 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.
Alex Christensen
Comment 6 2014-06-27 13:39:01 PDT
Note You need to log in before you can comment on or make changes to this bug.