1266695 - Port typed object getprop stub to CacheIR

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

Loading from a typed object is a bit involved, but it still gets rid of some boilerplate:

  7 files changed, 150 insertions(+), 188 deletions(-)

Comment 1

[Eric Faust [:efaust]]

Comment on attachment 8744278 [details] [diff] [review]
Patch

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

::: js/src/jit/BaselineCacheIR.cpp
@@ +877,5 @@
> +BaselineCacheIRCompiler::emitLoadTypedObjectResult()
> +{
> +    Register obj = allocator.useRegister(masm, reader.objOperandId());
> +    AutoScratchRegister scratch1(allocator, masm);
> +    AutoScratchRegister scratch2(allocator, masm);

These two read very similarly, which makes them hard to disambiguate below.

Maybe call scratch1 "fieldBase", or something? That's still not great for the load usage below...

In general, as an aside, I wish there was a way to do scoped register aliasing, so that you could do

{
   AutoNamedRegister name(scratchReg);

   // direct references to scratchReg crash in this block
}

or something similar. Then we could do what BC loves doing, which is

{
  Register name = scratch;
}

but without the fear of smashing ourselves.

@@ +925,5 @@
> +            masm.tagValue(JSVAL_TYPE_STRING, scratch1, R0);
> +            break;
> +
> +          default:
> +            MOZ_CRASH();

please add a string here: "unknown ReferenceTypeDescr" or so

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3996d3dd01ad

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Eric Faust [:efaust] from comment #1)
> These two read very similarly, which makes them hard to disambiguate below.
> 
> Maybe call scratch1 "fieldBase", or something? That's still not great for
> the load usage below...

Yeah I don't think renaming makes it much clearer as they're used multiple times..

> In general, as an aside, I wish there was a way to do scoped register
> aliasing, so that you could do

The MacroAssemblers have a ScratchRegisterScope, we could do something like that. CacheIR helps a lot with scratch register allocation though.

> please add a string here: "unknown ReferenceTypeDescr" or so

Done.

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3996d3dd01ad