Bug 188415 – Yarr JIT should include annotations with dumpDisassembly=true

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

All of the other JSC related JITs intermingle the byte code / IR in the disassembler output.  The Yarr regular expression JIT should also annotate its output with the compiled pattern info.

[Michael Saboff]

<rdar://problem/43063283>

[Michael Saboff]

Created attachment 346794 [details]
Patch

[EWS Watchlist]

Attachment 346794 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/yarr/YarrDisassembler.h:96:  The parameter name "vectorOrder" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/yarr/YarrDisassembler.h:98:  The parameter name "vectorOrder" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 346794 [details]
Patch

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

r=me

> Source/JavaScriptCore/yarr/YarrDisassembler.h:77
> +    enum VectorOrder {

enum class?

> Source/JavaScriptCore/yarr/YarrDisassembler.h:109
> +    unsigned m_indentLevel;

Making `m_identLevel { 0 };` looks nice to me instead of initializing it in a constructor.

> Source/JavaScriptCore/yarr/YarrJIT.cpp:45
> +//    friend void jitCompile(VM*, YarrCodeBlock&, const String& pattern, unsigned& numSubpatterns, const char*& error, bool ignoreCase, bool multiline);

Let's remove this.

[Michael Saboff]

(In reply to Yusuke Suzuki from comment #4)
> Comment on attachment 346794 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=346794&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/yarr/YarrDisassembler.h:77
> > +    enum VectorOrder {
> 
> enum class?

Done.

> > Source/JavaScriptCore/yarr/YarrDisassembler.h:109
> > +    unsigned m_indentLevel;
> 
> Making `m_identLevel { 0 };` looks nice to me instead of initializing it in
> a constructor.

Done.
 
> > Source/JavaScriptCore/yarr/YarrJIT.cpp:45
> > +//    friend void jitCompile(VM*, YarrCodeBlock&, const String& pattern, unsigned& numSubpatterns, const char*& error, bool ignoreCase, bool multiline);
> 
> Let's remove this.

Removed.  This was a holdover when I was checking if this was dead.

[Michael Saboff]

Committed r234713: <https://trac.webkit.org/changeset/234713>