1130910 - Fix various non-ion breakages

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Steve Singer (:stevensn)]

Fix some issues preventing non ion builds from compiling.


Bug 1125236 introduced a reference to the 'Operator' this in CodeGenerator-shared-inl.h so we need to add a stub implementation to the none jit.

I am also switching the Register and FloatRegister type to be a enum (like the other backends use) instead of an int. 

This patch also fixes a few other non-ion breakages that have recently showed up.

JitStackAlignment (Bug 1112159)
generateProfilerExitFrameTailStub (Bug 1057082 )

Comment 1

[Steve Singer (:stevensn)]

Attachment: 1130910_ionfixes.diff

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8561146 [details] [diff] [review]
1130910_ionfixes.diff

Review of attachment 8561146 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but some nits below.

::: js/src/jit/none/Architecture-none.h
@@ +20,5 @@
>  
>  class Registers
>  {
>    public:
> +    enum RegisterID { 

Nit: remove whitespace at the end of this line.

::: js/src/jit/none/MacroAssembler-none.h
@@ +140,5 @@
> +class VFPAddr {
> +};
> +
> +class DTRAddr {
> +};

These classes are very ARM-specific. Can you just add an empty Operand class or is that not enough?

@@ +505,5 @@
>      void loadAsmJSHeapRegisterFromGlobalData() { MOZ_CRASH(); }
>      void memIntToValue(Address, Address) { MOZ_CRASH(); }
>  
>      void setPrinter(Sprinter *) { MOZ_CRASH(); }
> +    Operand ToPayload(Operand base) { MOZ_CRASH(); } 

Nit: remove trailing whitespace.

@@ +509,5 @@
> +    Operand ToPayload(Operand base) { MOZ_CRASH(); } 
> +
> +    // Instrumentation for entering and leaving the profiler.
> +    void profilerEnterFrame(Register , Register ) { MOZ_CRASH(); }
> +    void profilerExitFrame() { MOZ_CRASH(); } 

And here.

@@ +519,5 @@
>  };
>  
>  typedef MacroAssemblerNone MacroAssemblerSpecific;
>  
> +

Nit: don't add a newline here.

@@ +537,5 @@
>  };
>  
> +
> +
> +

And here (two newlines).

::: js/src/jit/none/Trampoline-none.cpp
@@ +59,5 @@
>  bool ICCompare_Int32::Compiler::generateStubCode(MacroAssembler &) { MOZ_CRASH(); }
>  bool ICCompare_Double::Compiler::generateStubCode(MacroAssembler &) { MOZ_CRASH(); }
>  bool ICBinaryArith_Int32::Compiler::generateStubCode(MacroAssembler &) { MOZ_CRASH(); }
>  bool ICUnaryArith_Int32::Compiler::generateStubCode(MacroAssembler &) { MOZ_CRASH(); }
> +JitCode * JitRuntime::generateProfilerExitFrameTailStub(JSContext *) { MOZ_CRASH(); }

Nit: remove the space between '*' and JitRuntime.

Comment 3

[Steve Singer (:stevensn)]

Attachment: 1130910_ionfixes.diff

The Operand class needs Address & constructor.
Other than that it can be empty(at least for now).

I think I've addressed the rest of your concerns.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8563214 [details] [diff] [review]
1130910_ionfixes.diff

Review of attachment 8563214 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: js/src/jit/none/MacroAssembler-none.h
@@ +135,5 @@
>  
> +class Operand
> +{
> +public:
> +    Operand (const Address &) { MOZ_CRASH();}

Nit: indent 'public:' with two spaces, and remove space between 'Operand' and '('

Comment 5

[Steve Singer (:stevensn)]

Attachment: 1130910_ionfixes.diff

fixed indentation.

Comment 6

[Steve Singer (:stevensn)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fa555f6fced

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d0e9ab07f41

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d0e9ab07f41