[Webkit-unassigned] [Bug 91052] JSC: LLInt should auto-generate our cross-platform C++ interpreter

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91052





--- Comment #17 from Filip Pizlo <fpizlo at apple.com>  2012-08-16 14:24:22 PST ---
(In reply to comment #16)
> > OFFLINE_ASM_OPCODE_LABEL(op_create_this)
> >     r0.i = *CAST<intptr_t*>(fp.i8p - 32);                      // this<t0> = cfr.Callee;
> >     r2.i = *CAST<intptr_t*>(r0.i8p + 40);                      // inheritor<t2> = this<t0>.cachedInheritorID;
> 
> I see a couple issues here.
> 
> (1) r0 and r2 are declared to be interpreter-global. This will defeat local register allocation, possibly causing performance problems. Can you test the performance of this interpreter, as compared to the existing .cpp interpreter, and post the results?
> 
> Performance is not a top priority here, but we'd like to pay it some mind.
> 
> (2) This code is a bit cryptic. Assembly forces you to be cryptic. But we're auto-generating C++ here, so we can be much more explanatory. For example, when the existing .cpp interpreter wants to read the callee register, it just says "JSFunction* constructor = jsCast<JSFunction*>(callFrame->callee())". Why is that kind of code not possible here? Are we going out of our way to mimic the assembly generated by our assembly back-ends for some reason?

If you're generating C++ code from assembly code, I'm not sure how you're going to avoid it being cryptic.  In particular, the offlineasm blows away offsetof operations early, before it gets to the backend.  We could change that, but that would add an entirely separate path to the offlineasm and hence would defeat the goal of reducing the amount of code we want to maintain.

As well, I don't think we'll ever be touching the C++ code generated by the offlineasm, just as we currently almost never touch the assembly code generated by the offlineasm.

> 
> (3) Comments. I tend to agree that this code is cryptic, and could use some explanation. However, the comments are equally cryptic. I don't know what <> annotation means, and I don't know what these numbered t's are. 
> 
> The original code reads:
> 
>     loadp Callee[cfr], t0
> 
> Why can't our C++ code have variables named "cfr", "callee" and "t0", and perform an operation like "Register t0 = cfr[callee]"?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.