With JM and YARR gone, most of js/src/assembler is dead code. Some parts like the ExecutableAllocator and X86Assembler (but not the macro-assembler) are still used, these could probably be moved into jit/

I thought I'd give this a go. I managed to remove TestMain.cpp, LinkBuffer.h and RepatchBuffer.h, but I'm struggling beyond that. MacroAssembler is very much used by the JITs, and it depends on pretty much the rest of the code... maybe I have misunderstood comment 0. Any additional pointers to where the dead code may lie would be welcome!

Comment on attachment 8446887 [details] [diff] [review] Remove some dead code in js/src/assembler/assembler/ Review of attachment 8446887 [details] [diff] [review]: ----------------------------------------------------------------- assembler/MacroAssemblerX86.h etc should also be unused AFAIK. Where do we still depend on it? The macro-assemblers we use (jit/x86/MacroAssembler-x86.h etc) use some of the low-level JSC assembler code, but shouldn't depend on the MacroAssembler stuff AFAIK...

JSC::MacroAssembler is still used in the JITs. But now that I look more closely, it's only for a handful of things: Label, isSSE{2,3,41}Present(), getSSEState(), supportsFloatingPoint(), and SetSSE{3,4}Disabled(). So I should be able to remove almost everything in MacroAssembler.

(In reply to Nicholas Nethercote [:njn] from comment #3) > JSC::MacroAssembler is still used in the JITs. But now that I look more > closely, it's only for a handful of things: Label, isSSE{2,3,41}Present(), > getSSEState(), supportsFloatingPoint(), and SetSSE{3,4}Disabled(). So I > should be able to remove almost everything in MacroAssembler. We also have our own Label (jit/Label.h), but maybe the JSC one is used as well. Also, ARM and MIPS use the JSC assembler code even less, so you can probably remove more code there. You can build a 32-bit shell and add --enable-arm-simulator to build the ARM backend.

> We also have our own Label (jit/Label.h), but maybe the JSC one is used as > well. If you just grep for "JSC::MacroAssembler" you can see the handful of cases. > Also, ARM and MIPS use the JSC assembler code even less, so you can probably > remove more code there. You can build a 32-bit shell and add > --enable-arm-simulator to build the ARM backend. Yes, it looks like all the remaining uses are x86/x86-64 only. Thanks for the simulator tip!

> We also have our own Label (jit/Label.h), but maybe the JSC one is used as > well. Turns out all four instances are dead variables! :)

Who are the people who know about the ARMv7, MIPS and Sparc ports? I can't test those platforms and I'd like the relevant people to test my patch before it lands.

This patch deletes lots of dead code from js/src/assembler/. Platform-specific assemblers: - assembler/assembler/ARMAssembler.{h,cpp} - assembler/assembler/ARMv7Assembler.h - assembler/assembler/MIPSAssembler.h - assembler/assembler/SparcAssembler.h But X86Assembler.h is still used heavily. I didn't try to trim that file down at all. Other platform-independent, assembler-related stuff: - assembler/assembler/AbstractMacroAssembler.h - assembler/assembler/AssemblerBufferWithConstantPool.h - assembler/assembler/CodeLocation.h - assembler/assembler/LinkBuffer.h - assembler/assembler/MacroAssemblerCodeRef.h - assembler/assembler/RepatchBuffer.h And some miscellanea: - assembler/TestMain.cpp - assembler/moco/MocoStubs.h - assembler/wtf/SegmentedVector.h |MacroAssembler| still exists, and is typedef'd to the platform-specific implementations, but it's now tiny: it contains supportsFloatingPoint(), and on x86 also has the SSE detection methods; everything else has been removed. And MacroAssemblerBase has been merged into MacroAssembler. It compiles on x86, x86-64, and the ARM simulator. It needs testing on ARMv7, Sparc and MIPS. I haven't done anything about moving the remaining code into js/src/jit/. I figure that can be a follow-up. Diff stats: 29 files changed, 18 insertions(+), 18915 deletions(-)

(In reply to Nicholas Nethercote [:njn] from comment #7) > Who are the people who know about the ARMv7, MIPS and Sparc ports? I can't > test those platforms and I'd like the relevant people to test my patch > before it lands. ARMv7 is what the ARM simulator uses; as long as you have a green Try push that includes Android ARMv6 I think you're okay. needinfo? from Marty for the ARM ode and Branislav for MIPS.

Comment on attachment 8447788 [details] [diff] [review] Remove lots of dead code in js/src/assembler/ Review of attachment 8447788 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Nicholas Nethercote [:njn] from comment #8) > Diff stats: 29 files changed, 18 insertions(+), 18915 deletions(-) \o/ Great, thanks for doing this!

ARMv7 should be totally unused. IIRC, that is the name for the thumb assembler, which was never hooked up. It was probably the most confusing name for JSC to choose.

I can test your patch on MIPS. Just let me know when it is ready. I think that MIPS only uses supportsFloatingPoint from this folder. You can probably move this to jit folder.

(In reply to Branislav Rankov [:rankov] from comment #12) > I can test your patch on MIPS. Just let me know when it is ready. It's ready now! https://bugzilla.mozilla.org/attachment.cgi?id=8447788 Thanks :)

(In reply to Nicholas Nethercote [:njn] from comment #13) > It's ready now! https://bugzilla.mozilla.org/attachment.cgi?id=8447788 I have run the jit-tests with --tbpl on MIPS simulator. All the tests pass.

> I have run the jit-tests with --tbpl on MIPS simulator. All the tests pass. Thanks! My try server run was green, so I think this is ready to land. I can live without having tested Sparc in advance.

The try run in question: https://tbpl.mozilla.org/?tree=Try&rev=addc3a4d552a The Windows failures are not caused by this patch.