Bug 129228 – Compile attribute value matching

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

Compile attribute value matching

[Benjamin Poulain]

Created attachment 224995 [details]
Patch

[Geoffrey Garen]

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?

[Benjamin Poulain]

Committed r164852: <http://trac.webkit.org/changeset/164852>