| Summary: | Compile attribute value matching | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | allan.jensen, barraclough, cmarcelo, commit-queue, esprehn+autocc, ggaren, glenn, gyuyoung.kim, kangil.han, kling, macpherson, menard | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Benjamin Poulain
2014-02-23 01:16:00 PST
Created attachment 224995 [details]
Patch
Comment on attachment 224995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224995&action=review r=me > Source/WebCore/cssjit/FunctionCall.h:85 > + // x86 can swap without side effect. On other platforms, we need to be careful with the registers we use. I think you mean "swap without a temporary register". On other platforms, we need to allocate a register, and avoid conflicts with other allocated registers. > Source/WebCore/cssjit/FunctionCall.h:99 > + JSC::MacroAssembler::RegisterID freeRegister; > + for (unsigned i = 0; i < m_registerAllocator.allocatedRegisters().size(); ++i) { It looks funny, at first glance, to assign from something called "allocated registers" into something called "freeRegister". how about renaming "freeRegister" to "pushedRegister"? > Source/WebCore/cssjit/SelectorCompiler.cpp:401 > +static inline bool attributeValueTestingRequiresExtraRegister(const AttributeMatchingInfo& attributeInfo) Should we call this "attributeValueTestingRequiresCaseFoldingRegister"? > Source/WebCore/cssjit/SelectorCompiler.cpp:1172 > + // We make the assumption that name matching fails in most case and we keep value matching outside "cases" > Source/WebCore/cssjit/SelectorCompiler.cpp:1189 > + if (attributeSelector.m_match == CSSSelector::Exact) Can we make this an ASSERT? Committed r164852: <http://trac.webkit.org/changeset/164852> |