Summary: | Autogenerate yarr character tables | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric, loki, ossy, pvarga, webkit-ews, webkit.review.bot, zherczeg | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Oliver Hunt
2010-04-20 11:38:31 PDT
Created attachment 53858 [details]
Patch
Attachment 53858 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/assembler/X86Assembler.h:780: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/assembler/X86Assembler.h:911: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 13 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 53858 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1821004 Created attachment 53862 [details]
Patch
Attachment 53862 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/assembler/X86Assembler.h:780: cmpb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/assembler/X86Assembler.h:911: testb_im is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 53862 [details]
Patch
Please comment the CharacterClass implementation to explain that all classes have ranges; the table is optional.
Ideally I guess we should land the assembler changes first & get the arm port before relying on them – but you should at least give the arm jit devs a heads up.
Thank you Gavin letting us know! You are really kind! As far as I see the normal Address and ExtendedAddress will be the same on ARM since both intptr_t and int are four bytes long. Hence we can simply reuse the 32 bit versions. (In reply to comment #7) > Thank you Gavin letting us know! You are really kind! As far as I see the > normal Address and ExtendedAddress will be the same on ARM since both intptr_t > and int are four bytes long. Hence we can simply reuse the 32 bit versions. Can you come on irc? we do need additional instructions -- cmpb and testb for BaseIndex addresses I noticed that you didn't updated Yarr Interpreter for this feature. Thus the JavaScriptCore can't be compiled with Yarr Interpreter, because the linker can't link the functions of builtin CharacterClasses. ./libjscore.a(RegexInterpreter.o): In function `JSC::Yarr::ByteCompiler::compile()': RegexInterpreter.cpp:(.text._ZN3JSC4Yarr12ByteCompiler7compileEv[JSC::Yarr::ByteCompiler::compile()]+0x5e8): undefined reference to `JSC::Yarr::newlineCreate()' RegexInterpreter.cpp:(.text._ZN3JSC4Yarr12ByteCompiler7compileEv[JSC::Yarr::ByteCompiler::compile()]+0x633): undefined reference to `JSC::Yarr::wordcharCreate()' ... (In reply to comment #9) > I noticed that you didn't updated Yarr Interpreter for this feature. > Thus the JavaScriptCore can't be compiled with Yarr Interpreter, because > the linker can't link the functions of builtin CharacterClasses. > > ./libjscore.a(RegexInterpreter.o): In function > `JSC::Yarr::ByteCompiler::compile()': > RegexInterpreter.cpp:(.text._ZN3JSC4Yarr12ByteCompiler7compileEv[JSC::Yarr::ByteCompiler::compile()]+0x5e8): > undefined reference to `JSC::Yarr::newlineCreate()' > RegexInterpreter.cpp:(.text._ZN3JSC4Yarr12ByteCompiler7compileEv[JSC::Yarr::ByteCompiler::compile()]+0x633): > undefined reference to `JSC::Yarr::wordcharCreate()' > ... Fixed locally, but still not sure about what to do about the ARM backend as i'm not sure how i can avoid build breakage without someone else doing the code :-( (In reply to comment #10) > Fixed locally, but still not sure about what to do about the ARM backend as i'm > not sure how i can avoid build breakage without someone else doing the code :-( I think Gábor Zoltán or Gábor will submit the ARM patch asap, they will start their day at 11pm in PST. Committed r57925: <http://trac.webkit.org/changeset/57925> (In reply to comment #11) > (In reply to comment #10) > > Fixed locally, but still not sure about what to do about the ARM backend as i'm > > not sure how i can avoid build breakage without someone else doing the code :-( > > I think Gábor Zoltán or Gábor will submit the ARM > patch asap, they will start their day at 11pm in PST. Gabor tells me everything builds on arm fine so have landed. *crosses fingers* http://trac.webkit.org/changeset/57925 might have broken Leopard Intel Release (Tests) The following changes are on the blame list: http://trac.webkit.org/changeset/57924 http://trac.webkit.org/changeset/57925 |