Bug 129785

Summary: Use the LLVM disassembler on ARM64 if we are enabling the FTL
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, cmarcelo, commit-queue, ggaren, joepeck, mark.lam, mhahnenberg, mmirman, msaboff, nrotem, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 112840    
Attachments:
Description Flags
the patch none

Description Filip Pizlo 2014-03-05 23:50:16 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2014-03-05 23:52:14 PST
Created attachment 225959 [details]
the patch
Comment 2 Geoffrey Garen 2014-03-06 00:40:16 PST
Comment on attachment 225959 [details]
the patch

r=me
Comment 3 WebKit Commit Bot 2014-03-06 01:11:35 PST
Comment on attachment 225959 [details]
the patch

Clearing flags on attachment: 225959

Committed r165180: <http://trac.webkit.org/changeset/165180>
Comment 4 WebKit Commit Bot 2014-03-06 01:11:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Joseph Pecoraro 2014-03-06 02:08:43 PST
Building on my Mac I'm seeing a duplicate symbol Ld error:

duplicate symbol __ZN3JSC16tryToDisassembleERKNS_21MacroAssemblerCodePtrEmPKcRN3WTF11PrintStreamENS_21InstructionSubsetHintE in:
    /Volumes/Data/Build/JavaScriptCore.build/Debug/JavaScriptCore.build/Objects-normal/x86_64/ARM64Disassembler.o
    /Volumes/Data/Build/JavaScriptCore.build/Debug/JavaScriptCore.build/Objects-normal/x86_64/X86Disassembler.o

I'll try a clean build.
Comment 6 Joseph Pecoraro 2014-03-06 02:34:32 PST
Comment on attachment 225959 [details]
the patch

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

> Source/JavaScriptCore/disassembler/ARM64Disassembler.cpp:58
>  #endif // USE(ARM64_DISASSEMBLER)
>  
> +#if USE(LLVM_DISASSEMBLER)

I'm guessing this new code should probably have gone inside of the ARM64_DISASSEMBLER #ifdef. Moving it inside gets me past my duplicate symbol issue. A clean build did not help.
Comment 7 Filip Pizlo 2014-03-06 07:40:37 PST
Land patch, pass out!

I'm fixing it.
Comment 8 Filip Pizlo 2014-03-06 07:49:40 PST
Fixed in r165191